From f53fede65d6f15fa916b9c3ac370a73a95cf4791 Mon Sep 17 00:00:00 2001 From: Reed Loden Date: Mon, 30 Jul 2012 13:45:32 -0700 Subject: Bug 767623 - Use HMAC to generate tokens and sensitive graph filenames [r=LpSolit a=LpSolit] --- Bugzilla/Token.pm | 14 ++++++-------- reports.cgi | 14 ++++++-------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm index feb707e70..264a28db1 100644 --- a/Bugzilla/Token.pm +++ b/Bugzilla/Token.pm @@ -24,7 +24,7 @@ use Bugzilla::User; use Date::Format; use Date::Parse; use File::Basename; -use Digest::MD5 qw(md5_hex); +use Digest::SHA qw(hmac_sha256_base64); use base qw(Exporter); @@ -167,15 +167,13 @@ sub issue_hash_token { my $user_id = Bugzilla->user->id || remote_ip(); # The concatenated string is of the form - # token creation time + site-wide secret + user ID (either ID or remote IP) + data - my @args = ($time, Bugzilla->localconfig->{'site_wide_secret'}, $user_id, @$data); + # token creation time + user ID (either ID or remote IP) + data + my @args = ($time, $user_id, @$data); my $token = join('*', @args); - # Wide characters cause md5_hex() to die. - if (Bugzilla->params->{'utf8'}) { - utf8::encode($token) if utf8::is_utf8($token); - } - $token = md5_hex($token); + $token = hmac_sha256_base64($token, Bugzilla->localconfig->{'site_wide_secret'}); + $token =~ s/\+/-/g; + $token =~ s/\//_/g; # Prepend the token creation time, unencrypted, so that the token # lifetime can be validated. diff --git a/reports.cgi b/reports.cgi index 12087c852..66f4b05d7 100755 --- a/reports.cgi +++ b/reports.cgi @@ -17,7 +17,7 @@ use Bugzilla::Error; use Bugzilla::Status; use File::Basename; -use Digest::MD5 qw(md5_hex); +use Digest::SHA qw(hmac_sha256_base64); # If we're using bug groups for products, we should apply those restrictions # to viewing reports, as well. Time to check the login in that case. @@ -88,14 +88,12 @@ else { # Filenames must not be guessable as they can point to products # you are not allowed to see. Also, different projects can have # the same product names. - my $key = Bugzilla->localconfig->{'site_wide_secret'}; my $project = bz_locations()->{'project'} || ''; - my $image_file = join(':', ($key, $project, $prod_id, @datasets)); - # Wide characters cause md5_hex() to die. - if (Bugzilla->params->{'utf8'}) { - utf8::encode($image_file) if utf8::is_utf8($image_file); - } - $image_file = md5_hex($image_file) . '.png'; + my $image_file = join(':', ($project, $prod_id, @datasets)); + my $key = Bugzilla->localconfig->{'site_wide_secret'}; + $image_file = hmac_sha256_base64($image_file, $key) . '.png'; + $image_file =~ s/\+/-/g; + $image_file =~ s/\//_/g; trick_taint($image_file); if (! -e "$graph_dir/$image_file") { -- cgit v1.2.3-24-g4f1b