Skip to content

Commit

Permalink
fix vulnerability in yucky consent code
Browse files Browse the repository at this point in the history
  • Loading branch information
davidpanderson committed Dec 16, 2024
1 parent 14f7546 commit 8e564fa
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 21 deletions.
32 changes: 20 additions & 12 deletions html/inc/consent.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)) ) {
Expand All @@ -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);
Expand Down
19 changes: 10 additions & 9 deletions html/user/user_agreetermsofuse_action.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'])) {
Expand All @@ -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");
}


Expand Down

0 comments on commit 8e564fa

Please sign in to comment.