From 2c93f0a98f0f6380fd07ea17fd16afa2c6e4925b Mon Sep 17 00:00:00 2001 From: canyonknight Date: Sat, 23 Jun 2012 14:40:11 -0400 Subject: Implement token system to fix CSRF vulnerabilities Specially crafted pages can force authenticated users to unknowingly perform actions on the AUR website despite being on an attacker's website. This cross-site request forgery (CSRF) vulnerability applies to all POST data on the AUR. Implement a token system using a double submit cookie. Have a hidden form value on every page containing POST forms. Use the newly added check_token() to verify the token sent via POST matches the "AURSID" cookie value. Random nature of the token limits potential for CSRF. Signed-off-by: canyonknight Signed-off-by: Lukas Fleischer --- web/html/account.php | 12 +++++---- web/html/addvote.php | 7 ++++- web/html/packages.php | 70 ++++++++++++++++++++++++++------------------------ web/html/pkgsubmit.php | 14 +++++++--- web/html/tu.php | 2 +- 5 files changed, 61 insertions(+), 44 deletions(-) (limited to 'web/html') diff --git a/web/html/account.php b/web/html/account.php index 387fd93..5a0ef50 100644 --- a/web/html/account.php +++ b/web/html/account.php @@ -88,11 +88,13 @@ if (isset($_COOKIE["AURSID"])) { } elseif ($action == "UpdateAccount") { # user is submitting their modifications to an existing account # - process_account_form($atype, "edit", "UpdateAccount", - in_request("U"), in_request("T"), in_request("S"), - in_request("E"), in_request("P"), in_request("C"), - in_request("R"), in_request("L"), in_request("I"), - in_request("ID")); + if (check_token()) { + process_account_form($atype, "edit", "UpdateAccount", + in_request("U"), in_request("T"), in_request("S"), + in_request("E"), in_request("P"), in_request("C"), + in_request("R"), in_request("L"), in_request("I"), + in_request("ID")); + } } else { diff --git a/web/html/addvote.php b/web/html/addvote.php index f0e7d31..a5ec4a1 100644 --- a/web/html/addvote.php +++ b/web/html/addvote.php @@ -16,7 +16,11 @@ if (isset($_COOKIE["AURSID"])) { if ($atype == "Trusted User" OR $atype == "Developer") { $dbh = db_connect(); - if (!empty($_POST['addVote'])) { + if (!empty($_POST['addVote']) && !check_token()) { + $error = __("Invalid token for user action."); + } + + if (!empty($_POST['addVote']) && check_token()) { $error = ""; if (!empty($_POST['user'])) { @@ -99,6 +103,7 @@ if ($atype == "Trusted User" OR $atype == "Developer") {

+' /> ' />

diff --git a/web/html/packages.php b/web/html/packages.php index dc06c7e..7f31d3d 100644 --- a/web/html/packages.php +++ b/web/html/packages.php @@ -36,46 +36,48 @@ if (isset($_POST['IDs'])) { # Determine what action to do $output = ""; -if (current_action("do_Flag")) { - $output = pkg_flag($atype, $ids, true); -} elseif (current_action("do_UnFlag")) { - $output = pkg_flag($atype, $ids, False); -} elseif (current_action("do_Adopt")) { - $output = pkg_adopt($atype, $ids, true); -} elseif (current_action("do_Disown")) { - $output = pkg_adopt($atype, $ids, False); -} elseif (current_action("do_Vote")) { - $output = pkg_vote($atype, $ids, true); -} elseif (current_action("do_UnVote")) { - $output = pkg_vote($atype, $ids, False); -} elseif (current_action("do_Delete")) { - if (isset($_POST['confirm_Delete'])) { - if (!isset($_POST['merge_Into']) || empty($_POST['merge_Into'])) { - $output = pkg_delete($atype, $ids, NULL); - unset($_GET['ID']); - } - else { - $mergepkgid = pkgid_from_name($_POST['merge_Into']); - if ($mergepkgid) { - $output = pkg_delete($atype, $ids, $mergepkgid); +if (check_token()) { + if (current_action("do_Flag")) { + $output = pkg_flag($atype, $ids, true); + } elseif (current_action("do_UnFlag")) { + $output = pkg_flag($atype, $ids, False); + } elseif (current_action("do_Adopt")) { + $output = pkg_adopt($atype, $ids, true); + } elseif (current_action("do_Disown")) { + $output = pkg_adopt($atype, $ids, False); + } elseif (current_action("do_Vote")) { + $output = pkg_vote($atype, $ids, true); + } elseif (current_action("do_UnVote")) { + $output = pkg_vote($atype, $ids, False); + } elseif (current_action("do_Delete")) { + if (isset($_POST['confirm_Delete'])) { + if (!isset($_POST['merge_Into']) || empty($_POST['merge_Into'])) { + $output = pkg_delete($atype, $ids, NULL); unset($_GET['ID']); } else { - $output = __("Cannot find package to merge votes and comments into."); + $mergepkgid = pkgid_from_name($_POST['merge_Into']); + if ($mergepkgid) { + $output = pkg_delete($atype, $ids, $mergepkgid); + unset($_GET['ID']); + } + else { + $output = __("Cannot find package to merge votes and comments into."); + } } } + else { + $output = __("The selected packages have not been deleted, check the confirmation checkbox."); + } + } elseif (current_action("do_Notify")) { + $output = pkg_notify($atype, $ids); + } elseif (current_action("do_UnNotify")) { + $output = pkg_notify($atype, $ids, False); + } elseif (current_action("do_DeleteComment")) { + $output = pkg_delete_comment($atype); + } elseif (current_action("do_ChangeCategory")) { + $output = pkg_change_category($atype); } - else { - $output = __("The selected packages have not been deleted, check the confirmation checkbox."); - } -} elseif (current_action("do_Notify")) { - $output = pkg_notify($atype, $ids); -} elseif (current_action("do_UnNotify")) { - $output = pkg_notify($atype, $ids, False); -} elseif (current_action("do_DeleteComment")) { - $output = pkg_delete_comment($atype); -} elseif (current_action("do_ChangeCategory")) { - $output = pkg_change_category($atype); } html_header($title); diff --git a/web/html/pkgsubmit.php b/web/html/pkgsubmit.php index 539f056..c566cb4 100644 --- a/web/html/pkgsubmit.php +++ b/web/html/pkgsubmit.php @@ -27,9 +27,16 @@ if ($uid): if (isset($_REQUEST['pkgsubmit'])) { + # Make sure authenticated user submitted the package themselves + if (!check_token()) { + $error = __("Invalid token for user action."); + } + # Before processing, make sure we even have a file - if ($_FILES['pfile']['size'] == 0){ - $error = __("Error - No file uploaded"); + if (!$error) { + if ($_FILES['pfile']['size'] == 0){ + $error = __("Error - No file uploaded"); + } } # Check whether the file is gzip'ed @@ -448,7 +455,8 @@ html_header("Submit"); ?>
-
+
+ ' />
diff --git a/web/html/tu.php b/web/html/tu.php index 6e202c8..59cac1a 100644 --- a/web/html/tu.php +++ b/web/html/tu.php @@ -72,7 +72,7 @@ if ($atype == "Trusted User" OR $atype == "Developer") { } if ($canvote == 1) { - if (isset($_POST['doVote'])) { + if (isset($_POST['doVote']) && check_token()) { if (isset($_POST['voteYes'])) { $myvote = "Yes"; } else if (isset($_POST['voteNo'])) { -- cgit v1.2.3-54-g00ecf
: