summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDylan William Hardison <dylan@hardison.net>2018-01-03 20:22:04 +0100
committerDylan William Hardison <dylan@hardison.net>2018-01-04 14:13:12 +0100
commit09e1bbfee2f997261d24acb37d95bdb638467c02 (patch)
treec56e7b931edb0c918f8cc8f8c10e5435338fd46e
parent51605fb0ae3ce7d85b6037e0ac4b22676766ad0c (diff)
downloadbugzilla-09e1bbfee2f997261d24acb37d95bdb638467c02.tar.gz
bugzilla-09e1bbfee2f997261d24acb37d95bdb638467c02.tar.xz
Bug 1426409 - github_secret key has no rate limiting
-rw-r--r--Bugzilla.pm4
-rw-r--r--Bugzilla/Config.pm14
-rw-r--r--Bugzilla/Config/Admin.pm14
-rwxr-xr-xgithub.cgi26
4 files changed, 42 insertions, 16 deletions
diff --git a/Bugzilla.pm b/Bugzilla.pm
index 9e5177839..b7aaadf3f 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -317,7 +317,7 @@ sub github_secret {
my $cache = $class->request_cache;
my $cgi = $class->cgi;
- $cache->{github_secret} //= $cgi->cookie('github_secret') // generate_random_password(16);
+ $cache->{github_secret} //= $cgi->cookie('github_secret') // generate_random_password(256);
return $cache->{github_secret};
}
@@ -853,7 +853,7 @@ sub check_rate_limit {
$action = 'ignore';
}
my $limit = join("/", @$limit);
- Bugzilla->audit("[rate_limit] action=$action, ip=$ip, limit=$limit");
+ Bugzilla->audit("[rate_limit] action=$action, ip=$ip, limit=$limit, name=$name");
ThrowUserError("rate_limit") if $action eq 'block';
}
}
diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm
index f93992f91..d050ff9e0 100644
--- a/Bugzilla/Config.pm
+++ b/Bugzilla/Config.pm
@@ -193,6 +193,20 @@ sub update_params {
$param->{$name} = $item->{'default'};
}
}
+ else {
+ my $checker = $item->{'checker'};
+ my $updater = $item->{'updater'};
+ if ($checker) {
+ my $error = $checker->($param->{$name}, $item);
+ if ($error && $updater) {
+ my $new_val = $updater->( $param->{$name} );
+ $param->{$name} = $new_val unless $checker->($new_val, $item);
+ }
+ elsif ($error) {
+ warn "Invalid parameter: $name\n";
+ }
+ }
+ }
}
# Generate unique Duo integration secret key
diff --git a/Bugzilla/Config/Admin.pm b/Bugzilla/Config/Admin.pm
index ad24f7112..ac1c4ca0e 100644
--- a/Bugzilla/Config/Admin.pm
+++ b/Bugzilla/Config/Admin.pm
@@ -12,7 +12,7 @@ use strict;
use warnings;
use Bugzilla::Config::Common;
-use JSON::XS qw(decode_json);
+use JSON::XS qw(decode_json encode_json);
use List::MoreUtils qw(all);
use Scalar::Util qw(looks_like_number);
@@ -55,8 +55,9 @@ sub get_param_list {
{
name => 'rate_limit_rules',
type => 'l',
- default => '{"get_bug": [75, 60], "show_bug": [75, 60]}',
+ default => '{"get_bug": [75, 60], "show_bug": [75, 60], "github": [10, 60]}',
checker => \&check_rate_limit_rules,
+ updater => \&update_rate_limit_rules,
},
{
@@ -78,11 +79,18 @@ sub check_rate_limit_rules {
ref($_) eq 'ARRAY' && looks_like_number( $_->[0] ) && looks_like_number( $_->[1] )
} values %$val;
- foreach my $required (qw( show_bug get_bug )) {
+ foreach my $required (qw( show_bug get_bug github )) {
return "missing $required" unless exists $val->{$required};
}
return "";
}
+sub update_rate_limit_rules {
+ my ($rules) = @_;
+ my $val = decode_json($rules);
+ $val->{github} = [10, 60];
+ return encode_json($val);
+}
+
1;
diff --git a/github.cgi b/github.cgi
index b8467e1e0..f280f6ac9 100755
--- a/github.cgi
+++ b/github.cgi
@@ -13,7 +13,7 @@ use warnings;
use lib qw(. lib local/lib/perl5);
use Bugzilla;
-use Bugzilla::Util ();
+use Bugzilla::Util qw(remote_ip);
use Bugzilla::Error;
use Bugzilla::Constants;
use Bugzilla::Token qw( issue_short_lived_session_token
@@ -37,8 +37,10 @@ if (lc($cgi->request_method) eq 'post') {
my $github_secret = $cgi->param('github_secret') or ThrowCodeError("github_invalid_request", { reason => 'invalid secret' });
my $github_secret2 = Bugzilla->github_secret or ThrowCodeError("github_invalid_request", { reason => 'invalid secret' });
- ThrowCodeError("github_invalid_request", { reason => 'invalid secret' })
- unless $github_secret eq $github_secret2;
+ if ($github_secret ne $github_secret2) {
+ Bugzilla->check_rate_limit('github', remote_ip());
+ ThrowCodeError("github_invalid_request", { reason => 'invalid secret' });
+ }
ThrowCodeError("github_invalid_target", { target_uri => $target_uri })
unless $target_uri =~ /^\Q$urlbase\E/;
@@ -71,13 +73,18 @@ elsif (lc($cgi->request_method) eq 'get') {
exit;
}
- ThrowCodeError("github_invalid_request", { reason => 'invalid state param' })
- unless $state_param eq $state_cookie;
+ my $invalid_request = $state_param ne $state_cookie;
- my $state_data = get_token_extra_data($state_param);
- ThrowCodeError("github_invalid_request", { reason => 'invalid state param' } )
- unless $state_data && $state_data->{type};
+ my $state_data;
+ unless ($invalid_request) {
+ $state_data = get_token_extra_data($state_param);
+ $invalid_request = !( $state_data && $state_data->{type} && $state_data->{type} =~ /^github_(?:login|email)$/ );
+ }
+ if ($invalid_request) {
+ Bugzilla->check_rate_limit('github', remote_ip());
+ ThrowCodeError("github_invalid_request", { reason => 'invalid state param' } )
+ }
$cgi->remove_cookie('github_state');
delete_token($state_param);
@@ -90,9 +97,6 @@ elsif (lc($cgi->request_method) eq 'get') {
Bugzilla->request_cache->{github_action} = 'email';
Bugzilla->request_cache->{github_emails} = $state_data->{emails};
}
- else {
- ThrowCodeError("github_invalid_request", { reason => "invalid state param" })
- }
my $user = Bugzilla->login(LOGIN_REQUIRED);
my $target_uri = URI->new($state_data->{target_uri});