aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Bugzilla/Auth/Verify.pm8
-rw-r--r--Bugzilla/Token.pm1
-rw-r--r--Bugzilla/User.pm6
-rw-r--r--Bugzilla/Util.pm5
-rwxr-xr-xeditusers.cgi7
-rwxr-xr-xtoken.cgi40
-rwxr-xr-xuserprefs.cgi2
7 files changed, 37 insertions, 32 deletions
diff --git a/Bugzilla/Auth/Verify.pm b/Bugzilla/Auth/Verify.pm
index 52cebb5ea..deb5f4e95 100644
--- a/Bugzilla/Auth/Verify.pm
+++ b/Bugzilla/Auth/Verify.pm
@@ -77,6 +77,11 @@ sub create_or_update_user {
|| return { failure => AUTH_ERROR,
error => 'auth_invalid_email',
details => {addr => $username} };
+ # Usually we'd call validate_password, but external authentication
+ # systems might follow different standards than ours. So in this
+ # place here, we call trick_taint without checks.
+ trick_taint($password);
+
# XXX Theoretically this could fail with an error, but the fix for
# that is too involved to be done right now.
my $user = Bugzilla::User->create({
@@ -111,9 +116,6 @@ sub create_or_update_user {
validate_email_syntax($username)
|| return { failure => AUTH_ERROR, error => 'auth_invalid_email',
details => {addr => $username} };
- # Username is more than likely tainted, but we only use it in a
- # placeholder, and we've already validated it, so it's safe.
- trick_taint($username);
$dbh->do('UPDATE profiles SET login_name = ? WHERE userid = ?',
undef, $username, $user->id);
}
diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm
index a0f6b0c8e..051514b01 100644
--- a/Bugzilla/Token.pm
+++ b/Bugzilla/Token.pm
@@ -59,7 +59,6 @@ sub issue_new_user_account_token {
# an error because the user may have lost his email with the token inside.
# But to prevent using this way to mailbomb an email address, make sure
# the last request is at least 10 minutes old before sending a new email.
- trick_taint($login_name);
my $pending_requests =
$dbh->selectrow_array('SELECT COUNT(*)
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 02f17b85d..33c8535f5 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -1490,7 +1490,8 @@ sub is_available_username {
sub login_to_id {
my ($login, $throw_error) = @_;
my $dbh = Bugzilla->dbh;
- # $login will only be used by the following SELECT statement, so it's safe.
+ # No need to validate $login -- it will be used by the following SELECT
+ # statement only, so it's safe to simply trick_taint.
trick_taint($login);
my $user_id = $dbh->selectrow_array("SELECT userid FROM profiles WHERE " .
$dbh->sql_istrcmp('login_name', '?'),
@@ -1525,6 +1526,8 @@ sub validate_password {
} elsif ((defined $matchpassword) && ($password ne $matchpassword)) {
ThrowUserError('passwords_dont_match');
}
+ # Having done these checks makes us consider the password untainted.
+ trick_taint($_[0]);
return 1;
}
@@ -1966,6 +1969,7 @@ we return an empty string.
Returns true if a password is valid (i.e. meets Bugzilla's
requirements for length and content), else returns false.
+Untaints C<$passwd1> if successful.
If a second password is passed in, this function also verifies that
the two passwords match.
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index d346d2547..4a87ff042 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -456,6 +456,10 @@ sub validate_email_syntax {
my ($addr) = @_;
my $match = Bugzilla->params->{'emailregexp'};
my $ret = ($addr =~ /$match/ && $addr !~ /[\\\(\)<>&,;:"\[\] \t\r\n]/);
+ if ($ret) {
+ # We assume these checks to suffice to consider the address untainted.
+ trick_taint($_[0]);
+ }
return $ret ? 1 : 0;
}
@@ -893,6 +897,7 @@ and tokens.
Do a syntax checking for a legal email address and returns 1 if
the check is successful, else returns 0.
+Untaints C<$email> if successful.
=item C<validate_date($date)>
diff --git a/editusers.cgi b/editusers.cgi
index 19e7ea587..5f356fb40 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -257,14 +257,13 @@ if ($action eq 'search') {
my @values;
if ($login ne $otherUser->login) {
- # Validate, then trick_taint.
+ # Validating untaints for us.
$login || ThrowUserError('user_login_required');
validate_email_syntax($login)
|| ThrowUserError('illegal_email_address', {addr => $login});
is_available_username($login)
|| ThrowUserError('account_exists', {email => $login});
- trick_taint($login);
push(@changedFields, 'login_name');
push(@values, $login);
$logoutNeeded = 1;
@@ -280,9 +279,8 @@ if ($action eq 'search') {
push(@values, $realname);
}
if ($password) {
- # Validate, then trick_taint.
+ # Validating untaints for us.
validate_password($password) if $password;
- trick_taint($password);
push(@changedFields, 'cryptpassword');
push(@values, bz_crypt($password));
$logoutNeeded = 1;
@@ -296,7 +294,6 @@ if ($action eq 'search') {
$logoutNeeded = 1;
}
if ($disable_mail != $otherUser->email_disabled) {
- trick_taint($disable_mail);
push(@changedFields, 'disable_mail');
push(@values, $disable_mail);
}
diff --git a/token.cgi b/token.cgi
index 282d2fcbb..e45f49fba 100755
--- a/token.cgi
+++ b/token.cgi
@@ -64,9 +64,8 @@ if ($cgi->param('t')) {
$::token = $cgi->param('t');
# Make sure the token contains only valid characters in the right amount.
- # Validate password will throw an error if token is invalid
+ # validate_password will throw an error if token is invalid
validate_password($::token);
- trick_taint($::token); # Only used in placeholders
Bugzilla::Token::CleanTokenTable();
@@ -102,8 +101,10 @@ if ($cgi->param('t')) {
# If the user is requesting a password change, make sure they submitted
# their login name and it exists in the database, and that the DB module is in
# the list of allowed verification methods.
+my $login_name;
if ( $::action eq 'reqpw' ) {
- defined $cgi->param('loginname')
+ $login_name = $cgi->param('loginname');
+ defined $login_name
|| ThrowUserError("login_needed_for_password_change");
# check verification methods
@@ -111,27 +112,25 @@ if ( $::action eq 'reqpw' ) {
ThrowUserError("password_change_requests_not_allowed");
}
- # Make sure the login name looks like an email address.
- validate_email_syntax($cgi->param('loginname'))
- || ThrowUserError('illegal_email_address',
- {addr => $cgi->param('loginname')});
+ validate_email_syntax($login_name)
+ || ThrowUserError('illegal_email_address', {addr => $login_name});
- my $loginname = $cgi->param('loginname');
- trick_taint($loginname); # Used only in a placeholder
my ($user_id) = $dbh->selectrow_array('SELECT userid FROM profiles WHERE ' .
$dbh->sql_istrcmp('login_name', '?'),
- undef, $loginname);
+ undef, $login_name);
$user_id || ThrowUserError("account_inexistent");
}
# If the user is changing their password, make sure they submitted a new
# password and that the new password is valid.
+my $password;
if ( $::action eq 'chgpw' ) {
- defined $cgi->param('password')
+ $password = $cgi->param('password');
+ defined $password
&& defined $cgi->param('matchpassword')
|| ThrowUserError("require_new_password");
- validate_password($cgi->param('password'), $cgi->param('matchpassword'));
+ validate_password($password, $cgi->param('matchpassword'));
}
################################################################################
@@ -143,13 +142,13 @@ if ( $::action eq 'chgpw' ) {
# that variable and runs the appropriate code.
if ($::action eq 'reqpw') {
- requestChangePassword();
+ requestChangePassword($login_name);
} elsif ($::action eq 'cfmpw') {
confirmChangePassword();
} elsif ($::action eq 'cxlpw') {
cancelChangePassword();
} elsif ($::action eq 'chgpw') {
- changePassword();
+ changePassword($password);
} elsif ($::action eq 'cfmem') {
confirmChangeEmail();
} elsif ($::action eq 'cxlem') {
@@ -176,7 +175,8 @@ exit;
################################################################################
sub requestChangePassword {
- Bugzilla::Token::IssuePasswordToken($cgi->param('loginname'));
+ my ($login_name) = @_;
+ Bugzilla::Token::IssuePasswordToken($login_name);
$vars->{'message'} = "password_change_request";
@@ -203,11 +203,11 @@ sub cancelChangePassword {
}
sub changePassword {
+ my ($password) = @_;
my $dbh = Bugzilla->dbh;
# Create a crypted version of the new password
- my $cryptedpassword = bz_crypt($cgi->param('password'));
- trick_taint($cryptedpassword); # Used only in a placeholder
+ my $cryptedpassword = bz_crypt($password);
# Get the user's ID from the tokens table.
my ($userid) = $dbh->selectrow_array('SELECT userid FROM tokens
@@ -369,13 +369,13 @@ sub request_create_account {
sub confirm_create_account {
my (undef, undef, $login_name) = Bugzilla::Token::GetTokenData($::token);
- validate_password($cgi->param('passwd1') || '',
- $cgi->param('passwd2') || '');
+ my $password = $cgi->param('passwd1') || '';
+ validate_password($password, $cgi->param('passwd2') || '');
my $otheruser = Bugzilla::User->create({
login_name => $login_name,
realname => $cgi->param('realname'),
- cryptpassword => $cgi->param('passwd1')});
+ cryptpassword => $password});
# Now delete this token.
delete_token($::token);
diff --git a/userprefs.cgi b/userprefs.cgi
index d06e486ef..e8a045c4e 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -100,7 +100,6 @@ sub SaveAccount {
if ($cgi->param('Bugzilla_password') ne $pwd1) {
my $cryptedpassword = bz_crypt($pwd1);
- trick_taint($cryptedpassword); # Only used in a placeholder
$dbh->do(q{UPDATE profiles
SET cryptpassword = ?
WHERE userid = ?},
@@ -129,7 +128,6 @@ sub SaveAccount {
# Before changing an email address, confirm one does not exist.
validate_email_syntax($new_login_name)
|| ThrowUserError('illegal_email_address', {addr => $new_login_name});
- trick_taint($new_login_name);
is_available_username($new_login_name)
|| ThrowUserError("account_exists", {email => $new_login_name});