summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDave Lawrence <dlawrence@mozilla.com>2013-10-16 18:05:10 +0200
committerDave Lawrence <dlawrence@mozilla.com>2013-10-16 18:05:10 +0200
commit3771585c730f31f36a5efa3bd6b053ddf66bb2ba (patch)
tree7e01b7252cf9246d00ae56caa0db014121483258
parentf3b17d9f5351d9eca8d2c7f0feb272432fc398c9 (diff)
downloadbugzilla-3771585c730f31f36a5efa3bd6b053ddf66bb2ba.tar.gz
bugzilla-3771585c730f31f36a5efa3bd6b053ddf66bb2ba.tar.xz
Bug 906745 - In MySQL, tokens are not case-sensitive, reducing total entropy and allowing easier brute force
r=LpSolit,a=glob
-rw-r--r--Bugzilla/Token.pm22
-rw-r--r--template/en/default/global/code-error.html.tmpl3
-rwxr-xr-xtoken.cgi7
3 files changed, 24 insertions, 8 deletions
diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm
index 2bb68e721..9c2242f63 100644
--- a/Bugzilla/Token.pm
+++ b/Bugzilla/Token.pm
@@ -275,13 +275,18 @@ sub Cancel {
# Get information about the token being canceled.
trick_taint($token);
- my ($issuedate, $tokentype, $eventdata, $userid) =
- $dbh->selectrow_array('SELECT ' . $dbh->sql_date_format('issuedate') . ',
+ my ($db_token, $issuedate, $tokentype, $eventdata, $userid) =
+ $dbh->selectrow_array('SELECT token, ' . $dbh->sql_date_format('issuedate') . ',
tokentype, eventdata, userid
FROM tokens
WHERE token = ?',
undef, $token);
+ # Some DBs such as MySQL are case-insensitive by default so we do
+ # a quick comparison to make sure the tokens are indeed the same.
+ (defined $db_token && $db_token eq $token)
+ || ThrowCodeError("cancel_token_does_not_exist");
+
# If we are canceling the creation of a new user account, then there
# is no entry in the 'profiles' table.
my $user = new Bugzilla::User($userid);
@@ -346,10 +351,17 @@ sub GetTokenData {
$token = clean_text($token);
trick_taint($token);
- return $dbh->selectrow_array(
- "SELECT userid, " . $dbh->sql_date_format('issuedate') . ", eventdata
- FROM tokens
+ my @token_data = $dbh->selectrow_array(
+ "SELECT token, userid, " . $dbh->sql_date_format('issuedate') . ", eventdata
+ FROM tokens
WHERE token = ?", undef, $token);
+
+ # Some DBs such as MySQL are case-insensitive by default so we do
+ # a quick comparison to make sure the tokens are indeed the same.
+ my $db_token = shift @token_data;
+ return undef if (!defined $db_token || $db_token ne $token);
+
+ return @token_data;
}
# Deletes specified token
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 24e46fb14..877fe8d66 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -438,6 +438,9 @@
[% ELSIF error == "token_generation_error" %]
Something is seriously wrong with the token generation system.
+ [% ELSIF error == "cancel_token_does_not_exist" %]
+ The token to be cancelled does not exist.
+
[% ELSIF error == "template_error" %]
[% template_error_msg FILTER html %]
diff --git a/token.cgi b/token.cgi
index 20870159a..ae9800d72 100755
--- a/token.cgi
+++ b/token.cgi
@@ -67,9 +67,10 @@ if ($token) {
trick_taint($token);
# Make sure the token exists in the database.
- my ($tokentype) = $dbh->selectrow_array('SELECT tokentype FROM tokens
- WHERE token = ?', undef, $token);
- $tokentype || ThrowUserError("token_does_not_exist");
+ my ($db_token, $tokentype) = $dbh->selectrow_array('SELECT token, tokentype FROM tokens
+ WHERE token = ?', undef, $token);
+ (defined $db_token && $db_token eq $token && $tokentype)
+ || ThrowUserError("token_does_not_exist");
# Make sure the token is the correct type for the action being taken.
if ( grep($action eq $_ , qw(cfmpw cxlpw chgpw)) && $tokentype ne 'password' ) {