Merge pull request #5962 from BOINC/dpa_consent

fix vulnerability in yucky consent code

Author: AenBleidd

html/inc/consent.inc

@@ -23,12 +23,14 @@ require_once("../inc/util.inc");
 
 define('CONSENT_TYPE_ENROLL','ENROLL');
 
-// Utility function to check the terms of use.
 function check_termsofuse() {
     return defined('TERMSOFUSE_FILE') and file_exists(TERMSOFUSE_FILE);
 }
 
-function consent_to_a_policy($user, $consent_type_id, $consent_flag, $consent_not_required, $source, $ctime = 0) {
+function consent_to_a_policy(
+    $user, $consent_type_id, $consent_flag, $consent_not_required,
+    $source, $ctime = 0
+) {
     $mys = BoincDb::escape_string($source);
     if ($ctime==0) {
         $mytime = $user->create_time;
@@ -54,12 +56,13 @@ function check_user_consent($user, $consent_name) {
     return FALSE;
 }
 
-// Checks to see if a particular consent_type name is in
-// available. Returns an array of format: (BOOLEAN, INTEGER). The
-// boolean is T/F depending on whether that consent_type exists, and
-// if checkenabled=TRUE, if the consent_type is enabled/available for
-// use. The integer is the consent_type_id- the id from consent_type
-// table. If the boolean is FALSE, the integer returned is -1.
+// Check if a particular consent_type name is available.
+// Returns an array of format: (BOOLEAN, INTEGER).
+// The boolean is T/F depending on whether that consent_type exists,
+// and if checkenabled=TRUE, if the consent_type is enabled/available for use.
+// The integer is the consent_type_id- the id from consent_type table.
+// If the boolean is FALSE, the integer returned is -1.
+//
 function check_consent_type($name, $checkenabled=TRUE) {
     $ct = BoincConsentType::lookup("shortname = '{$name}'");
     if ($ct and ( !$checkenabled or ($ct->enabled)) ) {
@@ -68,13 +71,18 @@ function check_consent_type($name, $checkenabled=TRUE) {
     return array(FALSE, -1);
 }
 
-// When a user uses the Web site to login, this funtion checks the
-// ENROLL consent and intercepts the login, presenting the terms of
-// use page Web form before they can continue.
+// When a user uses the Web site to login, this function checks the
+// ENROLL consent and intercepts the login,
+// presenting the terms of use page Web form before they can continue.
+//
 function intercept_login($user, $perm, $in_next_url = "") {
     list($checkct, $ctid) = check_consent_type(CONSENT_TYPE_ENROLL);
     $config = get_config();
-    if ( parse_bool($config, "enable_login_mustagree_termsofuse") and $checkct and check_termsofuse() and (!check_user_consent($user, CONSENT_TYPE_ENROLL))) {
+    if (parse_bool($config, "enable_login_mustagree_termsofuse")
+        and $checkct
+        and check_termsofuse()
+        and (!check_user_consent($user, CONSENT_TYPE_ENROLL))
+    ) {
         // sent user to terms-of-use Web form after login
         $mytoken = create_token($user->id, TOKEN_TYPE_LOGIN_INTERCEPT, TOKEN_DURATION_TWO_HOURS);
         send_cookie('logintoken', $mytoken, false);

html/user/user_agreetermsofuse_action.php

@@ -25,7 +25,7 @@
 require_once("../inc/consent.inc");
 
 if (empty($_POST)) {
-    error_page(tra("Website error when attempting to agree to terms of use. Please contact the site administrators."));
+    error_page("Missing args");
 }
 
 // Get the next url from POST
@@ -36,23 +36,25 @@
     $next_url = HOME_PAGE;
 }
 
-// validate checkbox
 $agree = post_str("agree_to_terms_of_use", true);
 if (!$agree) {
-    error_page(tra("You have not agreed to our terms of use. You may not continue until you do so."));
+    error_page(tra("Agree to terms of use to continue."));
 }
 
 // Obtain data from cookies
 if (isset($_COOKIE['logintoken'])) {
     $logintoken = $_COOKIE['logintoken'];
 } else {
-    error_page(tra("Website error when attempting to agree to terms of use."));
+    error_page("Missing arg");
 }
 
 if (isset($_COOKIE['tempuserid'])) {
     $userid = $_COOKIE['tempuserid'];
+    if (filter_var($userid, FILTER_VALIDATE_INT) === false) {
+        error_page("Bad arg");
+    }
 } else {
-    error_page(tra("Website error when attempting to agree to terms of use. Please contact the site administrators."));
+    error_page("Missing arg");
 }
 
 if (isset($_COOKIE['tempperm'])) {
@@ -66,22 +68,21 @@
 // misuse of the token.
 if (!is_valid_token($userid, $logintoken, TOKEN_TYPE_LOGIN_INTERCEPT)) {
     delete_token($userid, $logintoken, TOKEN_TYPE_LOGIN_INTERCEPT);
-    error_page(tra("Authentication error attempting to agree to terms of use."));
+    error_page("Invalid token");
 }
 delete_token($userid, $logintoken, TOKEN_TYPE_LOGIN_INTERCEPT);
 
 $user = BoincUser::lookup_id_nocache($userid);
 $authenticator = $user->authenticator;
 
-// Set CONSENT_TYPE_ENROLL in database.
 list($checkct, $ctid) = check_consent_type(CONSENT_TYPE_ENROLL);
 if ($checkct) {
     $rc1 = consent_to_a_policy($user, $ctid, 1, 0, 'Webform', time());
     if (!$rc1) {
-        error_page("Database error when attempting to INSERT into table consent with ID=$user->id. " . BoincDb::error() . " Please contact site administrators.");
+        error_page("Database error");
     }
 } else {
-    error_page("Error: consent type for enrollment not found. Please contact site administrators.");
+    error_page("Consent type not found");
 }