From 9f472d2693fa9447390f228d66372b85c3c12e90 Mon Sep 17 00:00:00 2001 From: flashwave Date: Mon, 18 Mar 2019 23:02:30 +0100 Subject: [PATCH] Stricter checking on GET variables. --- public/auth.php | 4 ++- public/auth/login.php | 2 +- public/changelog.php | 8 +++--- public/comments.php | 54 +++++++++++++++++++--------------------- public/forum/forum.php | 2 +- public/forum/index.php | 6 +++-- public/forum/post.php | 11 ++++---- public/forum/posting.php | 16 ++++++------ public/forum/topic.php | 15 +++++------ public/members.php | 6 ++--- public/news.php | 16 +++++++++--- public/profile.php | 12 +++++---- public/relations.php | 4 +-- public/user-assets.php | 4 +-- src/csrf.php | 6 ++++- src/pagination.php | 4 +++ 16 files changed, 96 insertions(+), 74 deletions(-) diff --git a/public/auth.php b/public/auth.php index a1288516..8571944c 100644 --- a/public/auth.php +++ b/public/auth.php @@ -3,7 +3,9 @@ require_once '../misuzu.php'; -switch ($_GET['m'] ?? '') { +$mode = !empty($_GET['m']) && is_string($_GET['m']) ? $_GET['m'] : ''; + +switch ($mode) { case 'logout': echo tpl_render('auth.logout'); break; diff --git a/public/auth/login.php b/public/auth/login.php index ed367138..f2ea9f7a 100644 --- a/public/auth/login.php +++ b/public/auth/login.php @@ -6,7 +6,7 @@ if (user_session_active()) { return; } -if (isset($_GET['resolve_user']) && is_string($_GET['resolve_user'])) { +if (!empty($_GET['resolve_user']) && is_string($_GET['resolve_user'])) { header('Content-Type: text/plain; charset=utf-8'); echo user_id_from_username($_GET['resolve_user']); return; diff --git a/public/changelog.php b/public/changelog.php index 321908e8..c917fcb2 100644 --- a/public/changelog.php +++ b/public/changelog.php @@ -1,10 +1,10 @@ 0) { header(csrf_http_header('comments')); $commentPerms = comments_get_perms($currentUserId); -switch ($_GET['m'] ?? null) { +$commentId = !empty($_GET['c']) && is_string($_GET['c']) ? (int)$_GET['c'] : 0; +$commentMode = !empty($_GET['m']) && is_string($_GET['m']) ? (string)$_GET['m'] : ''; +$commentVote = !empty($_GET['v']) && is_string($_GET['v']) ? (int)$_GET['v'] : MSZ_COMMENTS_VOTE_INDIFFERENT; + +switch ($commentMode) { case 'pin': case 'unpin': if (!$commentPerms['can_pin']) { @@ -45,8 +49,7 @@ switch ($_GET['m'] ?? null) { break; } - $comment = (int)($_GET['c'] ?? 0); - $commentInfo = comments_post_get($comment, false); + $commentInfo = comments_post_get($commentId, false); if (!$commentInfo || $commentInfo['comment_deleted'] !== null) { echo render_info_or_json($isXHR, "This comment doesn't exist!", 400); @@ -58,7 +61,7 @@ switch ($_GET['m'] ?? null) { break; } - $isPinning = $_GET['m'] === 'pin'; + $isPinning = $commentMode === 'pin'; if ($isPinning && !empty($commentInfo['comment_pinned'])) { echo render_info_or_json($isXHR, 'This comment is already pinned.', 400); @@ -87,15 +90,12 @@ switch ($_GET['m'] ?? null) { break; } - $vote = (int)($_GET['v'] ?? MSZ_COMMENTS_VOTE_INDIFFERENT); - - if (!comments_vote_type_valid($vote)) { + if (!comments_vote_type_valid($commentVote)) { echo render_info_or_json($isXHR, 'Invalid vote action.', 400); break; } - $comment = (int)($_GET['c'] ?? 0); - $commentInfo = comments_post_get($comment, false); + $commentInfo = comments_post_get($commentId, false); if (!$commentInfo || $commentInfo['comment_deleted'] !== null) { echo render_info_or_json($isXHR, "This comment doesn't exist!", 400); @@ -103,17 +103,17 @@ switch ($_GET['m'] ?? null) { } $voteResult = comments_vote_add( - $comment, + $commentInfo['comment_id'], user_session_current('user_id', 0), - $vote + $commentVote ); if (!$isXHR) { - header('Location: ' . $redirect . '#comment-' . $comment); + header('Location: ' . $redirect . '#comment-' . $commentInfo['comment_id']); break; } - echo json_encode(comments_votes_get($comment)); + echo json_encode(comments_votes_get($commentInfo['comment_id'])); break; case 'delete': @@ -122,8 +122,7 @@ switch ($_GET['m'] ?? null) { break; } - $comment = (int)($_GET['c'] ?? 0); - $commentInfo = comments_post_get($comment, false); + $commentInfo = comments_post_get($commentId, false); if (!$commentInfo) { echo render_info_or_json($isXHR, "This comment doesn't exist.", 400); @@ -147,19 +146,19 @@ switch ($_GET['m'] ?? null) { break; } - if (!comments_post_delete($comment)) { + if (!comments_post_delete($commentInfo['comment_id'])) { echo render_info_or_json($isXHR, 'Failed to delete comment.', 500); break; } if ($isModAction) { audit_log(MSZ_AUDIT_COMMENT_ENTRY_DELETE_MOD, $currentUserId, [ - $comment, + $commentInfo['comment_id'], (int)($commentInfo['user_id'] ?? 0), $commentInfo['username'] ?? '(Deleted User)', ]); } else { - audit_log(MSZ_AUDIT_COMMENT_ENTRY_DELETE, $currentUserId, [$comment]); + audit_log(MSZ_AUDIT_COMMENT_ENTRY_DELETE, $currentUserId, [$commentInfo['comment_id']]); } if ($redirect) { @@ -168,7 +167,7 @@ switch ($_GET['m'] ?? null) { } echo json_encode([ - 'id' => $comment, + 'id' => $commentInfo['comment_id'], ]); break; @@ -178,8 +177,7 @@ switch ($_GET['m'] ?? null) { break; } - $comment = (int)($_GET['c'] ?? 0); - $commentInfo = comments_post_get($comment, false); + $commentInfo = comments_post_get($commentId, false); if (!$commentInfo) { echo render_info_or_json($isXHR, "This comment doesn't exist.", 400); @@ -191,24 +189,24 @@ switch ($_GET['m'] ?? null) { break; } - if (!comments_post_delete($comment, false)) { + if (!comments_post_delete($commentInfo['comment_id'], false)) { echo render_info_or_json($isXHR, 'Failed to restore comment.', 500); break; } audit_log(MSZ_AUDIT_COMMENT_ENTRY_RESTORE, $currentUserId, [ - $comment, + $commentInfo['comment_id'], (int)($commentInfo['user_id'] ?? 0), $commentInfo['username'] ?? '(Deleted User)', ]); if ($redirect) { - header('Location: ' . $redirect . '#comment-' . $comment); + header('Location: ' . $redirect . '#comment-' . $commentInfo['comment_id']); break; } echo json_encode([ - 'id' => $comment, + 'id' => $commentInfo['comment_id'], ]); break; @@ -223,7 +221,7 @@ switch ($_GET['m'] ?? null) { break; } - $categoryId = (int)($_POST['comment']['category'] ?? 0); + $categoryId = !empty($_POST['comment']['category']) && is_string($_POST['comment']['category']) ? (int)$_POST['comment']['category'] : 0; $category = comments_category_info($categoryId); if (!$category) { @@ -236,10 +234,10 @@ switch ($_GET['m'] ?? null) { break; } - $commentText = $_POST['comment']['text'] ?? ''; + $commentText = !empty($_POST['comment']['text']) && is_string($_POST['comment']['text']) ? $_POST['comment']['text'] : ''; $commentLock = !empty($_POST['comment']['lock']) && $commentPerms['can_lock']; $commentPin = !empty($_POST['comment']['pin']) && $commentPerms['can_pin']; - $commentReply = (int)($_POST['comment']['reply'] ?? 0); + $commentReply = !empty($_POST['comment']['reply']) && is_string($_POST['comment']['reply']) ? (int)$_POST['comment']['reply'] : 0; if ($commentLock) { comments_category_lock($categoryId, is_null($category['category_locked'])); diff --git a/public/forum/forum.php b/public/forum/forum.php index 5f2e32bc..cfd84613 100644 --- a/public/forum/forum.php +++ b/public/forum/forum.php @@ -1,7 +1,7 @@ $postInfo['post_id'], 'post_fragment' => 'p' . $postInfo['post_id'], @@ -165,7 +166,7 @@ switch ($postMode) { } if (!$isXHR) { - if ($postRequestVerified && isset($_GET['confirm']) && $_GET['confirm'] !== '1') { + if ($postRequestVerified && !$submissionConfirmed) { header("Location: " . url('forum-post', [ 'post' => $postInfo['post_id'], 'post_fragment' => 'p' . $postInfo['post_id'], @@ -207,7 +208,7 @@ switch ($postMode) { } if (!$isXHR) { - if ($postRequestVerified && isset($_GET['confirm']) && $_GET['confirm'] !== '1') { + if ($postRequestVerified && !$submissionConfirmed) { header("Location: " . url('forum-post', [ 'post' => $postInfo['post_id'], 'post_fragment' => 'p' . $postInfo['post_id'], diff --git a/public/forum/posting.php b/public/forum/posting.php index a7cf66c2..52fd945e 100644 --- a/public/forum/posting.php +++ b/public/forum/posting.php @@ -16,15 +16,15 @@ $forumPostingModes = [ ]; if (!empty($_POST)) { - $mode = $_POST['post']['mode'] ?? 'create'; - $postId = max(0, (int)($_POST['post']['id'] ?? 0)); - $topicId = max(0, (int)($_POST['post']['topic'] ?? 0)); - $forumId = max(0, (int)($_POST['post']['forum'] ?? 0)); + $mode = !empty($_POST['post']['mode']) && is_string($_POST['post']['mode']) ? $_POST['post']['mode'] : 'create'; + $postId = !empty($_POST['post']['id']) && is_string($_POST['post']['id']) ? (int)$_POST['post']['id'] : 0; + $topicId = !empty($_POST['post']['topic']) && is_string($_POST['post']['topic']) ? (int)$_POST['post']['topic'] : 0; + $forumId = !empty($_POST['post']['forum']) && is_string($_POST['post']['forum']) ? (int)$_POST['post']['forum'] : 0; } else { - $mode = $_GET['m'] ?? 'create'; - $postId = max(0, (int)($_GET['p'] ?? 0)); - $topicId = max(0, (int)($_GET['t'] ?? 0)); - $forumId = max(0, (int)($_GET['f'] ?? 0)); + $mode = !empty($_GET['m']) && is_string($_GET['m']) ? $_GET['m'] : 'create'; + $postId = !empty($_GET['p']) && is_string($_GET['p']) ? (int)$_GET['p'] : 0; + $topicId = !empty($_GET['t']) && is_string($_GET['t']) ? (int)$_GET['t'] : 0; + $forumId = !empty($_GET['f']) && is_string($_GET['f']) ? (int)$_GET['f'] : 0; } if (!in_array($mode, $forumPostingModes, true)) { diff --git a/public/forum/topic.php b/public/forum/topic.php index 4fb30e5a..68ae990b 100644 --- a/public/forum/topic.php +++ b/public/forum/topic.php @@ -1,8 +1,10 @@ $topic['topic_id']] @@ -202,7 +203,7 @@ if (in_array($moderationMode, $validModerationModes, true)) { } if (!$isXHR) { - if (isset($_GET['confirm']) && $_GET['confirm'] !== '1') { + if (!$submissionConfirmed) { header("Location: " . url('forum-topic', [ 'topic' => $topic['topic_id'], ])); @@ -245,7 +246,7 @@ if (in_array($moderationMode, $validModerationModes, true)) { } if (!$isXHR) { - if (isset($_GET['confirm']) && $_GET['confirm'] !== '1') { + if (!$submissionConfirmed) { header('Location: ' . url('forum-topic', [ 'topic' => $topic['topic_id'], ])); diff --git a/public/members.php b/public/members.php index dc2f8db6..5c7b3e0e 100644 --- a/public/members.php +++ b/public/members.php @@ -1,9 +1,9 @@ 'Ascending', diff --git a/public/news.php b/public/news.php index e32bf610..b4492464 100644 --- a/public/news.php +++ b/public/news.php @@ -1,10 +1,18 @@ (int)$_GET['n'], + ])); + http_response_code(301); + return; +} -if ($postId !== null) { +$categoryId = !empty($_GET['c']) && is_string($_GET['c']) ? (int)$_GET['c'] : 0; +$postId = !empty($_GET['p']) && is_string($_GET['p']) ? (int)$_GET['p'] : 0; + +if ($postId > 0) { $post = news_post_get($postId); if (!$post) { @@ -35,7 +43,7 @@ if ($postId !== null) { return; } -if ($categoryId !== null) { +if ($categoryId > 0) { $category = news_category_get($categoryId, true); if (empty($category)) { diff --git a/public/profile.php b/public/profile.php index 93242510..1fa8887a 100644 --- a/public/profile.php +++ b/public/profile.php @@ -1,7 +1,11 @@ $profile, - 'profile_mode' => $mode, + 'profile_mode' => $profileMode, 'profile_notices' => $notices, 'profile_can_edit' => $canEdit, 'profile_is_editing' => $isEditing, diff --git a/public/relations.php b/public/relations.php index f223266c..08700ff8 100644 --- a/public/relations.php +++ b/public/relations.php @@ -32,8 +32,8 @@ if (user_warning_check_expiration($userId, MSZ_WARN_BAN) > 0) { return; } -$subjectId = (int)($_GET['u'] ?? 0); -$relationType = (int)($_GET['m'] ?? -1); +$subjectId = !empty($_GET['u']) && is_string($_GET['u']) ? (int)$_GET['u'] : 0; +$relationType = !empty($_GET['m']) && is_string($_GET['m']) ? (int)$_GET['m'] : -1; if (!user_relation_is_valid_type($relationType)) { echo render_info_or_json($isXHR, 'Invalid relation type.', 400); diff --git a/public/user-assets.php b/public/user-assets.php index e406eae0..38f8da9c 100644 --- a/public/user-assets.php +++ b/public/user-assets.php @@ -1,10 +1,10 @@