From 0e390970ba51b14a5dc780be7c6f0d6d7baa67e3 Mon Sep 17 00:00:00 2001 From: Frédéric Buclin Date: Thu, 17 Apr 2014 18:11:12 +0200 Subject: Bug 713926: (CVE-2014-1517) [SECURITY] Login form lacks CSRF protection r=dkl a=justdave --- Bugzilla/Auth/Login/CGI.pm | 41 +++++++++++++++++++++++++++++++++++++---- Bugzilla/Auth/Persist/Cookie.pm | 4 ++++ 2 files changed, 41 insertions(+), 4 deletions(-) (limited to 'Bugzilla/Auth') diff --git a/Bugzilla/Auth/Login/CGI.pm b/Bugzilla/Auth/Login/CGI.pm index a55275a54..f9e3bbf18 100644 --- a/Bugzilla/Auth/Login/CGI.pm +++ b/Bugzilla/Auth/Login/CGI.pm @@ -17,19 +17,52 @@ use Bugzilla::Constants; use Bugzilla::WebService::Constants; use Bugzilla::Util; use Bugzilla::Error; +use Bugzilla::Token; sub get_login_info { my ($self) = @_; my $params = Bugzilla->input_params; + my $cgi = Bugzilla->cgi; + + my $login = trim(delete $params->{'Bugzilla_login'}); + my $password = delete $params->{'Bugzilla_password'}; + # The token must match the cookie to authenticate the request. + my $login_token = delete $params->{'Bugzilla_login_token'}; + my $login_cookie = $cgi->cookie('Bugzilla_login_request_cookie'); - my $username = trim(delete $params->{"Bugzilla_login"}); - my $password = delete $params->{"Bugzilla_password"}; + my $valid = 0; + # If the web browser accepts cookies, use them. + if ($login_token && $login_cookie) { + my ($time, undef) = split(/-/, $login_token); + # Regenerate the token based on the information we have. + my $expected_token = issue_hash_token(['login_request', $login_cookie], $time); + $valid = 1 if $expected_token eq $login_token; + $cgi->remove_cookie('Bugzilla_login_request_cookie'); + } + # WebServices and other local scripts can bypass this check. + # This is safe because we won't store a login cookie in this case. + elsif (Bugzilla->usage_mode != USAGE_MODE_BROWSER) { + $valid = 1; + } + # Else falls back to the Referer header and accept local URLs. + # Attachments are served from a separate host (ideally), and so + # an evil attachment cannot abuse this check with a redirect. + elsif (my $referer = $cgi->referer) { + my $urlbase = correct_urlbase(); + $valid = 1 if $referer =~ /^\Q$urlbase\E/; + } + # If the web browser doesn't accept cookies and the Referer header + # is missing, we have no way to make sure that the authentication + # request comes from the user. + elsif ($login && $password) { + ThrowUserError('auth_untrusted_request', { login => $login }); + } - if (!defined $username || !defined $password) { + if (!$login || !$password || !$valid) { return { failure => AUTH_NODATA }; } - return { username => $username, password => $password }; + return { username => $login, password => $password }; } sub fail_nodata { diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm index 5a9857cba..6f4eac96d 100644 --- a/Bugzilla/Auth/Persist/Cookie.pm +++ b/Bugzilla/Auth/Persist/Cookie.pm @@ -54,6 +54,10 @@ sub persist_login { $dbh->bz_commit_transaction(); + # We do not want WebServices to generate login cookies. + # All we need is the login token for User.login. + return $login_cookie if i_am_webservice(); + # Prevent JavaScript from accessing login cookies. my %cookieargs = ('-httponly' => 1); -- cgit v1.2.3-24-g4f1b