summaryrefslogtreecommitdiffstats
path: root/extensions
diff options
context:
space:
mode:
authorDylan Hardison <dylan@mozilla.com>2015-11-05 06:28:14 +0100
committerDylan Hardison <dylan@mozilla.com>2015-11-05 06:28:14 +0100
commit534fc2123e40b7517aeaffd709faf72af97ac3b8 (patch)
tree18ad69c8fb22e213ee3256c0768e35dd964d2156 /extensions
parent67d9618771441215d8c431b81bf66acd4faa2aa1 (diff)
downloadbugzilla-534fc2123e40b7517aeaffd709faf72af97ac3b8.tar.gz
bugzilla-534fc2123e40b7517aeaffd709faf72af97ac3b8.tar.xz
Bug 1196743 - Fix information disclosure vulnerability that allows attacker to obtain victim's GitHub OAuth return code
Diffstat (limited to 'extensions')
-rw-r--r--extensions/GitHubAuth/Extension.pm15
-rw-r--r--extensions/GitHubAuth/lib/Client.pm44
-rw-r--r--extensions/GitHubAuth/lib/Login.pm91
-rw-r--r--extensions/GitHubAuth/lib/Util.pm35
-rw-r--r--extensions/GitHubAuth/template/en/default/hook/account/auth/login-additional_methods.html.tmpl10
-rw-r--r--extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl12
-rw-r--r--extensions/GitHubAuth/template/en/default/hook/global/code-error-errors.html.tmpl12
7 files changed, 77 insertions, 142 deletions
diff --git a/extensions/GitHubAuth/Extension.pm b/extensions/GitHubAuth/Extension.pm
index d68934be4..b813689cd 100644
--- a/extensions/GitHubAuth/Extension.pm
+++ b/extensions/GitHubAuth/Extension.pm
@@ -12,7 +12,6 @@ use strict;
use parent qw(Bugzilla::Extension);
use Bugzilla::Extension::GitHubAuth::Client;
-use Bugzilla::Extension::GitHubAuth::Util qw(target_uri);
use Bugzilla::Error;
use Bugzilla::Util qw(trick_taint);
@@ -30,7 +29,7 @@ BEGIN {
my ($stack, $method) = @_;
return undef if $method eq 'fail_nodata';
- return $stack->UNIVERSAL::can($method);
+ return $stack->SUPER::can($method);
};
}
@@ -42,18 +41,6 @@ sub install_before_final_checks {
}) unless Bugzilla::Group->new({ name => 'no-github-auth' });
}
-sub template_before_create {
- my ($self, $args) = @_;
-
- return if Bugzilla->user->id && !Bugzilla->cgi->param('logout');
-
- $args->{config}{VARIABLES}{github_auth} = {
- login => sub {
- return Bugzilla::Extension::GitHubAuth::Client->login_uri(target_uri());
- },
- };
-}
-
sub attachment_should_redirect_login {
my ($self, $args) = @_;
my $cgi = Bugzilla->cgi;
diff --git a/extensions/GitHubAuth/lib/Client.pm b/extensions/GitHubAuth/lib/Client.pm
index 338a5b639..77c8a6c61 100644
--- a/extensions/GitHubAuth/lib/Client.pm
+++ b/extensions/GitHubAuth/lib/Client.pm
@@ -15,7 +15,7 @@ use URI::QueryParam;
use Digest;
use Bugzilla::Extension::GitHubAuth::Client::Error qw(ThrowUserError ThrowCodeError);
-use Bugzilla::Util qw(remote_ip);
+use Bugzilla::Util qw(remote_ip correct_urlbase);
use constant DIGEST_HASH => 'SHA1';
@@ -35,19 +35,22 @@ sub new {
}
sub login_uri {
- my ($self, $target) = @_;
+ my ($class, $target_uri) = @_;
- $target->query_param(GoAheadAndLogIn => 1);
- $target->query_param(github_login => 1);
- $target->query_param_delete('logout');
+ my $uri = URI->new(correct_urlbase() . "github.cgi");
+ $uri->query_form(target_uri => $target_uri);
+ return $uri;
+}
- my $uri = URI->new(GH_AUTHORIZE_URI);
+sub authorize_uri {
+ my ($class, $state) = @_;
+ my $uri = URI->new(GH_AUTHORIZE_URI);
$uri->query_form(
client_id => Bugzilla->params->{github_client_id},
scope => 'user:email',
- state => $self->get_state($target),
- redirect_uri => $target,
+ state => $state,
+ redirect_uri => correct_urlbase() . "github.cgi",
);
return $uri;
@@ -65,31 +68,6 @@ sub get_email_key {
return $digest->hexdigest;
}
-sub get_state {
- my ($class, $target) = @_;
- my $sorted_target = $target->clone;
- $sorted_target->query_form({});
-
- foreach my $key (sort $target->query_param) {
- $sorted_target->query_param($key, $target->query_param($key));
- }
-
- $sorted_target->query_param_delete("code");
- $sorted_target->query_param_delete("state");
- $sorted_target->query_param_delete('github_email_key');
- $sorted_target->query_param_delete('github_email');
- $sorted_target->query_param_delete('GoAheadAndLogIn');
- $sorted_target->query_param_delete('github_login');
-
- my $cgi = Bugzilla->cgi;
- my $digest = Digest->new(DIGEST_HASH);
- $digest->add($sorted_target->as_string);
- $digest->add(remote_ip());
- $digest->add($cgi->cookie('Bugzilla_github_token') // Bugzilla->request_cache->{github_token} // '');
- $digest->add(Bugzilla->localconfig->{site_wide_secret});
- return $digest->hexdigest;
-}
-
sub _handle_response {
my ($self, $response) = @_;
my $data = eval {
diff --git a/extensions/GitHubAuth/lib/Login.pm b/extensions/GitHubAuth/lib/Login.pm
index 6e105c33a..f1effb4e1 100644
--- a/extensions/GitHubAuth/lib/Login.pm
+++ b/extensions/GitHubAuth/lib/Login.pm
@@ -13,11 +13,12 @@ use fields qw(github_failure);
use Scalar::Util qw(blessed);
-use Bugzilla::Constants qw(AUTH_NODATA AUTH_ERROR USAGE_MODE_BROWSER );
+use Bugzilla::Constants qw(AUTH_NODATA AUTH_ERROR USAGE_MODE_BROWSER);
use Bugzilla::Util qw(trick_taint correct_urlbase generate_random_password);
+use Bugzilla::Token qw(issue_short_lived_session_token set_token_extra_data);
+use List::MoreUtils qw(any);
use Bugzilla::Extension::GitHubAuth::Client;
use Bugzilla::Extension::GitHubAuth::Client::Error ();
-use Bugzilla::Extension::GitHubAuth::Util qw(target_uri);
use Bugzilla::Error;
use constant { requires_verification => 1,
@@ -27,30 +28,16 @@ use constant { requires_verification => 1,
sub get_login_info {
my ($self) = @_;
my $cgi = Bugzilla->cgi;
- my $github_login = $cgi->param('github_login');
- my $github_email = $cgi->param('github_email');
- my $github_email_key = $cgi->param('github_email_key');
-
- my $cookie = $cgi->cookie('Bugzilla_github_token');
- unless ($cookie) {
- my $token = generate_random_password();
- $cgi->send_cookie(-name => 'Bugzilla_github_token',
- -value => $token,
- Bugzilla->params->{'ssl_redirect'} ? ( -secure => 1 ) : (),
- -httponly => 1);
- Bugzilla->request_cache->{github_token} = $token;
- }
+ my $github_action = Bugzilla->request_cache->{github_action};
- return { failure => AUTH_NODATA } unless $github_login;
+ return { failure => AUTH_NODATA } unless $github_action;
my $response;
- if ($github_email_key && $github_email) {
- trick_taint($github_email);
- trick_taint($github_email_key);
- $response = $self->_get_login_info_from_email($github_email, $github_email_key);
+ if ($github_action eq 'login') {
+ $response = $self->_get_login_info_from_github();
}
- else {
- $response = $self->_get_login_info_from_github();
+ elsif ($github_action eq 'email') {
+ $response = $self->_get_login_info_from_email();
}
if (!exists $response->{failure}) {
@@ -84,20 +71,13 @@ sub _get_login_info_from_github {
my ($self) = @_;
my $cgi = Bugzilla->cgi;
my $template = Bugzilla->template;
- my $state = $cgi->param('state');
my $code = $cgi->param('code');
return { failure => AUTH_ERROR, error => 'github_missing_code' } unless $code;
- return { failure => AUTH_ERROR, error => 'github_invalid_state' } unless $state;
trick_taint($code);
- trick_taint($state);
- my $target = target_uri();
my $client = Bugzilla::Extension::GitHubAuth::Client->new;
- if ($state ne $client->get_state($target)) {
- return { failure => AUTH_ERROR, error => 'github_invalid_state' };
- }
my ($access_token, $emails);
eval {
@@ -119,15 +99,6 @@ sub _get_login_info_from_github {
my @emails = map { $_->{email} }
grep { $_->{verified} && $_->{email} !~ /\@users\.noreply\.github\.com$/ } @$emails;
- my $choose_email = sub {
- my ($email) = @_;
- my $uri = $target->clone;
- my $key = Bugzilla::Extension::GitHubAuth::Client->get_email_key($email);
- $uri->query_param(github_email => $email);
- $uri->query_param(github_email_key => $key);
- return $uri;
- };
-
my @bugzilla_users;
my @github_emails;
foreach my $email (@emails) {
@@ -143,7 +114,6 @@ sub _get_login_info_from_github {
if (@allowed_bugzilla_users == 1) {
my ($user) = @allowed_bugzilla_users;
- $cgi->remove_cookie('Bugzilla_github_token');
return { user => $user };
}
elsif (@allowed_bugzilla_users > 1) {
@@ -151,7 +121,7 @@ sub _get_login_info_from_github {
template => 'account/auth/github-verify-account.html.tmpl',
vars => {
bugzilla_users => \@allowed_bugzilla_users,
- choose_email => $choose_email,
+ choose_email => _mk_choose_email(\@emails),
},
};
return { failure => AUTH_NODATA };
@@ -165,7 +135,7 @@ sub _get_login_info_from_github {
template => 'account/auth/github-verify-account.html.tmpl',
vars => {
github_emails => \@github_emails,
- choose_email => $choose_email,
+ choose_email => _mk_choose_email(\@emails),
},
};
return { failure => AUTH_NODATA };
@@ -176,19 +146,22 @@ sub _get_login_info_from_github {
}
sub _get_login_info_from_email {
- my ($self, $github_email, $github_email_key) = @_;
+ my ($self) = @_;
my $cgi = Bugzilla->cgi;
+ my $email = $cgi->param('email') or return { failure => AUTH_ERROR,
+ user_error => 'github_invalid_email',
+ details => { email => '' } };
+ trick_taint($email);
- my $key = Bugzilla::Extension::GitHubAuth::Client->get_email_key($github_email);
- unless ($github_email_key eq $key) {
+ unless (any { $_ eq $email } @{ Bugzilla->request_cache->{github_emails} }) {
return { failure => AUTH_ERROR,
user_error => 'github_invalid_email',
- details => { email => $github_email }};
+ details => { email => $email }};
}
- my $user = Bugzilla::User->new({name => $github_email, cache => 1});
+ my $user = Bugzilla::User->new({name => $email, cache => 1});
$cgi->remove_cookie('Bugzilla_github_token');
- return $user ? { user => $user } : { email => $github_email };
+ return $user ? { user => $user } : { email => $email };
}
sub fail_nodata {
@@ -206,5 +179,29 @@ sub fail_nodata {
exit;
}
+sub _store_emails {
+ my ($emails) = @_;
+ my $state = issue_short_lived_session_token("github_email");
+ set_token_extra_data($state, { type => 'github_email',
+ emails => $emails,
+ target_uri => Bugzilla->request_cache->{github_target_uri} });
+
+ Bugzilla->cgi->send_cookie(-name => 'github_state',
+ -value => $state,
+ -httponly => 1);
+ return $state;
+}
+
+sub _mk_choose_email {
+ my ($emails) = @_;
+ my $state = _store_emails($emails);
+
+ return sub {
+ my $email = shift;
+ my $uri = URI->new(correct_urlbase() . "github.cgi");
+ $uri->query_form( state => $state, email => $email );
+ return $uri;
+ };
+}
1;
diff --git a/extensions/GitHubAuth/lib/Util.pm b/extensions/GitHubAuth/lib/Util.pm
deleted file mode 100644
index bda76a420..000000000
--- a/extensions/GitHubAuth/lib/Util.pm
+++ /dev/null
@@ -1,35 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Extension::GitHubAuth::Util;
-
-use strict;
-use warnings;
-
-use Bugzilla::Util qw(correct_urlbase);
-use URI;
-
-use base qw(Exporter);
-our @EXPORT = qw( target_uri );
-
-
-# this is like correct_urlbase() except it returns the *requested* uri, before http url rewrites have been applied.
-# needed to generate github's redirect_uri.
-sub target_uri {
- my $cgi = Bugzilla->cgi;
- my $base = URI->new(correct_urlbase());
- if (my $request_uri = $cgi->request_uri) {
- $base->path('');
- $request_uri =~ s!^/+!!;
- return URI->new($base . "/" . $request_uri);
- }
- else {
- return URI->new(correct_urlbase() . $cgi->url(-relative => 1, query => ));
- }
-}
-
-1;
diff --git a/extensions/GitHubAuth/template/en/default/hook/account/auth/login-additional_methods.html.tmpl b/extensions/GitHubAuth/template/en/default/hook/account/auth/login-additional_methods.html.tmpl
index 26eb8d63b..609b86159 100644
--- a/extensions/GitHubAuth/template/en/default/hook/account/auth/login-additional_methods.html.tmpl
+++ b/extensions/GitHubAuth/template/en/default/hook/account/auth/login-additional_methods.html.tmpl
@@ -5,14 +5,16 @@
# This Source Code Form is "Incompatible With Secondary Licenses", as
# defined by the Mozilla Public License, v. 2.0.
#%]
-
+[% USE Bugzilla %]
[% IF Param('user_info_class').split(',').contains('GitHubAuth') %]
<p>
- <a href="[% github_auth.login FILTER html %]">
- <img src="extensions/GitHubAuth/web/images/github_sign_in.png"
+ <form method="post" action="[% urlbase FILTER html %]github.cgi">
+ <input type="hidden" name="github_secret" value="[% Bugzilla.github_secret FILTER html %]">
+ <input type="hidden" name="target_uri" value="[% Bugzilla.cgi.target_uri FILTER html %]">
+ <input type="image" src="extensions/GitHubAuth/web/images/github_sign_in.png"
alt="Sign in with GitHub"
title="Sign in with GitHub"
width="185" height="25">
- </a>
+ </form>
</p>
[% END %]
diff --git a/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl b/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl
index 6c4582b70..f32b34a59 100644
--- a/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl
+++ b/extensions/GitHubAuth/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl
@@ -5,6 +5,7 @@
# This Source Code Form is "Incompatible With Secondary Licenses", as
# defined by the Mozilla Public License, v. 2.0.
#%]
+[% USE Bugzilla %]
[% IF Param('user_info_class').split(',').contains('GitHubAuth') %]
<script type="text/javascript">
@@ -18,9 +19,12 @@
});
</script>
<span id="github_mini_login[% qs_suffix FILTER html %]" class="bz_default_hidden">
- <a href="[% github_auth.login FILTER html %]">
- <img src="extensions/GitHubAuth/web/images/sign_in.png" height="22" width="75" align="absmiddle"
- alt="Sign in with GitHub"
- title="Sign in with GitHub"></a> or
+ <form method="post" action="[% urlbase FILTER html %]github.cgi">
+ <input type="hidden" name="github_secret" value="[% Bugzilla.github_secret FILTER html %]">
+ <input type="hidden" name="target_uri" value="[% Bugzilla.cgi.target_uri FILTER html %]">
+ <input type="image" src="extensions/GitHubAuth/web/images/sign_in.png" height="22" width="75" align="absmiddle"
+ alt="Sign in with GitHub"
+ title="Sign in with GitHub"></a> or
+ </form>
</span>
[% END %]
diff --git a/extensions/GitHubAuth/template/en/default/hook/global/code-error-errors.html.tmpl b/extensions/GitHubAuth/template/en/default/hook/global/code-error-errors.html.tmpl
index 5f6672e2b..aaf9b6fa3 100644
--- a/extensions/GitHubAuth/template/en/default/hook/global/code-error-errors.html.tmpl
+++ b/extensions/GitHubAuth/template/en/default/hook/global/code-error-errors.html.tmpl
@@ -6,11 +6,7 @@
# defined by the Mozilla Public License, v. 2.0.
#%]
-[% IF error == "github_invalid_state" %]
- [% title = "Invalid State Parameter" %]
- An invalid <em>state</em> parameter was passed to the GitHub OAuth2 callback.
-
-[% ELSIF error == "github_missing_code" %]
+[% IF error == "github_missing_code" %]
[% title = "Missing GitHub Auth Code" %]
Expected a <em>code</em> parameter in the GitHub OAuth2 callback.
@@ -22,4 +18,10 @@
[% title = "GitHub Error" %]
GitHub returned an error: [% response.message FILTER html %]
+[% ELSIF error == "github_invalid_target" %]
+ [% terms.Bugzilla %] cannot log you into an external site via GitHub.
+
+[% ELSIF error == "github_invalid_request" %]
+ Invalid GitHub log in attempt (reason: [% reason FILTER html %])
+
[% END %]