updated to 5.0.4

Author: himorin

Bugzilla/CGI.pm

@@ -288,6 +288,69 @@ sub close_standby_message {
     }
 }
 
+our $ALLOW_UNSAFE_RESPONSE = 0;
+# responding to text/plain or text/html is safe
+# responding to any request with a referer header is safe
+# some things need to have unsafe responses (attachment.cgi)
+# everything else should get a 403.
+sub _prevent_unsafe_response {
+    my ($self, $headers) = @_;
+    my $safe_content_type_re = qr{
+        ^ (*COMMIT) # COMMIT makes the regex faster
+                    # by preventing back-tracking. see also perldoc pelre.
+        # application/x-javascript, xml, atom+xml, rdf+xml, xml-dtd, and json
+        (?: application/ (?: x(?: -javascript | ml (?: -dtd )? )
+                           | (?: atom | rdf) \+ xml
+                           | json )
+        # text/csv, text/calendar, text/plain, and text/html
+          | text/ (?: c (?: alendar | sv )
+                    | plain
+                    | html )
+        # used for HTTP push responses
+          | multipart/x-mixed-replace)
+    }sx;
+    my $safe_referer_re = do {
+        # Note that urlbase must end with a /.
+        # It almost certainly does, but let's be extra careful.
+        my $urlbase = correct_urlbase();
+        $urlbase =~ s{/$}{};
+        qr{
+            # Begins with literal urlbase
+            ^ (*COMMIT)
+            \Q$urlbase\E
+            # followed by a slash or end of string
+            (?: /
+              | $ )
+        }sx
+    };
+
+    return if $ALLOW_UNSAFE_RESPONSE;
+
+    if (Bugzilla->usage_mode == USAGE_MODE_BROWSER) {
+        # Safe content types are ones that arn't images.
+        # For now let's assume plain text and html are not valid images.
+        my $content_type         = $headers->{'-type'} // $headers->{'-content_type'} // 'text/html';
+        my $is_safe_content_type = $content_type =~ $safe_content_type_re;
+
+        # Safe referers are ones that begin with the urlbase.
+        my $referer         = $self->referer;
+        my $is_safe_referer = $referer && $referer =~ $safe_referer_re;
+
+        if (!$is_safe_referer && !$is_safe_content_type) {
+            print $self->SUPER::header(-type => 'text/html',  -status => '403 Forbidden');
+            if ($content_type ne 'text/html') {
+                print "Untrusted Referer Header\n";
+                if ($ENV{MOD_PERL}) {
+                    my $r = $self->r;
+                    $r->rflush;
+                    $r->status(200);
+                }
+            }
+            exit;
+        }
+    }
+}
+
 # Override header so we can add the cookies in
 sub header {
     my $self = shift;
@@ -302,6 +365,7 @@ sub header {
     else {
         %headers = @_;
     }
+    $self->_prevent_unsafe_response(\%headers);
 
     if ($self->{'_content_disp'}) {
         $headers{'-content_disposition'} = $self->{'_content_disp'};

Bugzilla/Config.pm

@@ -16,10 +16,9 @@ use autodie qw(:default);
 
 use Bugzilla::Constants;
 use Bugzilla::Hook;
-use Bugzilla::Util qw(trick_taint);
+use Bugzilla::Util qw(trick_taint read_text write_text);
 
 use JSON::XS;
-use File::Slurp;
 use File::Temp;
 use File::Basename;
 
@@ -284,7 +283,7 @@ sub write_params {
     my $param_file = bz_locations()->{'datadir'} . '/params.json';
 
     my $json_data = JSON::XS->new->canonical->pretty->encode($param_data);
-    write_file($param_file, { binmode => ':utf8', atomic => 1 }, \$json_data);
+    write_text($param_file, $json_data);
 
     # It's not common to edit parameters and loading
     # Bugzilla::Install::Filesystem is slow.
@@ -301,8 +300,8 @@ sub read_param_file {
     my $file = bz_locations()->{'datadir'} . '/params.json';
 
     if (-e $file) {
-        my $data;
-        read_file($file, binmode => ':utf8', buf_ref => \$data);
+        my $data = read_text($file);
+        trick_taint($data);
 
         # If params.json has been manually edited and e.g. some quotes are
         # missing, we don't want JSON::XS to leak the content of the file

Bugzilla/Constants.pm

@@ -200,7 +200,7 @@ use Memoize;
 # CONSTANTS
 #
 # Bugzilla version
-use constant BUGZILLA_VERSION => "5.0.3";
+use constant BUGZILLA_VERSION => "5.0.4";
 
 # A base link to the current REST Documentation. We place it here
 # as it will need to be updated to whatever the current release is.

Bugzilla/DB/Sqlite.pm

@@ -219,6 +219,7 @@ sub sql_date_format {
     my ($self, $date, $format) = @_;
     $format = "%Y.%m.%d %H:%M:%S" if !$format;
     $format =~ s/\%i/\%M/g;
+    $format =~ s/\%s/\%S/g;
     return "STRFTIME(" . $self->quote($format) . ", $date)";
 }
 

Bugzilla/Install/Filesystem.pm

@@ -31,7 +31,6 @@ use File::Path;
 use File::Basename;
 use File::Copy qw(move);
 use File::Spec;
-use File::Slurp;
 use IO::File;
 use POSIX ();
 
@@ -536,7 +535,7 @@ sub update_filesystem {
 
     # Remove old assets htaccess file to force recreation with correct values.
     if (-e "$assetsdir/.htaccess") {
-        if (read_file("$assetsdir/.htaccess") =~ /<FilesMatch \\\.css\$>/) {
+        if (read_text("$assetsdir/.htaccess") =~ /<FilesMatch \\\.css\$>/) {
             unlink("$assetsdir/.htaccess");
         }
     }
@@ -782,31 +781,30 @@ sub _update_old_charts {
 # to product IDs.
 sub _update_old_mining_filenames {
     my ($miningdir) = @_;
+    my $dbh = Bugzilla->dbh;
     my @conversion_errors;
 
-    require Bugzilla::Product;
-
     # We use a dummy product instance with ID 0, representing all products
     my $product_all = {id => 0, name => '-All-'};
-    bless($product_all, 'Bugzilla::Product');
 
     print "Updating old charting data file names...";
-    my @products = Bugzilla::Product->get_all();
+    my @products = @{ $dbh->selectall_arrayref('SELECT id, name FROM products
+                                                ORDER BY name', {Slice=>{}}) };
     push(@products, $product_all);
     foreach my $product (@products) {
-        if (-e File::Spec->catfile($miningdir, $product->id)) {
+        if (-e File::Spec->catfile($miningdir, $product->{id})) {
             push(@conversion_errors,
                  { product => $product,
-                   message => 'A file named "' . $product->id .
+                   message => 'A file named "' . $product->{id} .
                               '" already exists.' });
         }
     }
 
     if (! @conversion_errors) {
         # Renaming mining files should work now without a hitch.
         foreach my $product (@products) {
-            if (! rename(File::Spec->catfile($miningdir, $product->name),
-                         File::Spec->catfile($miningdir, $product->id))) {
+            if (! rename(File::Spec->catfile($miningdir, $product->{name}),
+                         File::Spec->catfile($miningdir, $product->{id}))) {
                 push(@conversion_errors,
                      { product => $product,
                        message => $! });
@@ -822,7 +820,7 @@ sub _update_old_mining_filenames {
         print " FAILED:\n";
         foreach my $error (@conversion_errors) {
             printf "Cannot rename charting data file for product %d (%s): %s\n",
-                   $error->{product}->id, $error->{product}->name,
+                   $error->{product}->{id}, $error->{product}->{name},
                    $error->{message};
         }
         print "You need to empty the \"$miningdir\" directory, then run\n",

Bugzilla/Install/Requirements.pm

@@ -155,11 +155,6 @@ sub REQUIRED_MODULES {
         module  => 'Math::Random::ISAAC',
         version => '1.0.1',
     },
-    {
-        package => 'File-Slurp',
-        module  => 'File::Slurp',
-        version => '9999.13',
-    },
     {
         package => 'JSON-XS',
         module  => 'JSON::XS',

Bugzilla/JobQueue.pm

@@ -14,8 +14,8 @@ use warnings;
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Install::Util qw(install_string);
+use Bugzilla::Util qw(read_text);
 use File::Basename;
-use File::Slurp;
 use base qw(TheSchwartz);
 use fields qw(_worker_pidfile);
 
@@ -124,7 +124,7 @@ sub subprocess_worker {
             # And poll the PID to detect when the working has finished.
             # We do this instead of system() to allow for the INT signal to
             # interrup us and trigger kill_worker().
-            my $pid = read_file($self->{_worker_pidfile}, err_mode => 'quiet');
+            my $pid = read_text($self->{_worker_pidfile}, err_mode => 'quiet');
             if ($pid) {
                 sleep(3) while(kill(0, $pid));
             }
@@ -139,7 +139,7 @@ sub subprocess_worker {
 sub kill_worker {
     my $self = Bugzilla->job_queue();
     if ($self->{_worker_pidfile} && -e $self->{_worker_pidfile}) {
-        my $worker_pid = read_file($self->{_worker_pidfile});
+        my $worker_pid = read_text($self->{_worker_pidfile});
         if ($worker_pid && kill(0, $worker_pid)) {
             $self->debug("Stopping worker process");
             system "$0 -f -p '" . $self->{_worker_pidfile} . "' stop";

Bugzilla/Migrate.pm

@@ -403,7 +403,7 @@ sub parse_date {
     }
     my $tz;
     if ($time[6]) {
-        $tz = Bugzilla->local_timezone->offset_as_string($time[6]);
+        $tz = DateTime::TimeZone->offset_as_string($time[6]);
     }
     else {
         $tz = $self->config('timezone');

Bugzilla/Template.pm

@@ -32,7 +32,6 @@ use Digest::MD5 qw(md5_hex);
 use File::Basename qw(basename dirname);
 use File::Find;
 use File::Path qw(rmtree mkpath);
-use File::Slurp;
 use File::Spec;
 use IO::Dir;
 use List::MoreUtils qw(firstidx);
@@ -502,7 +501,7 @@ sub _concatenate_css {
         next unless -e "$cgi_path/$files{$source}";
         my $file = $skins_path . '/' . md5_hex($source) . '.css';
         if (!-e $file) {
-            my $content = read_file("$cgi_path/$files{$source}");
+            my $content = read_text("$cgi_path/$files{$source}");
 
             # minify
             $content =~ s{/\*.*?\*/}{}sg;   # comments
@@ -512,7 +511,7 @@ sub _concatenate_css {
             # rewrite urls
             $content =~ s{url\(([^\)]+)\)}{_css_url_rewrite($source, $1)}eig;
 
-            write_file($file, "/* $files{$source} */\n" . $content . "\n");
+            write_text($file, "/* $files{$source} */\n" . $content . "\n");
         }
         push @minified, $file;
     }
@@ -522,9 +521,9 @@ sub _concatenate_css {
     if (!-e $file) {
         my $content = '';
         foreach my $source (@minified) {
-            $content .= read_file($source);
+            $content .= read_text($source);
         }
-        write_file($file, $content);
+        write_text($file, $content);
     }
 
     $file =~ s/^\Q$cgi_path\E\///o;
@@ -563,7 +562,7 @@ sub _concatenate_js {
         next unless -e "$cgi_path/$files{$source}";
         my $file = $skins_path . '/' . md5_hex($source) . '.js';
         if (!-e $file) {
-            my $content = read_file("$cgi_path/$files{$source}");
+            my $content = read_text("$cgi_path/$files{$source}");
 
             # minimal minification
             $content =~ s#/\*.*?\*/##sg;    # block comments
@@ -572,7 +571,7 @@ sub _concatenate_js {
             $content =~ s#\n{2,}#\n#g;      # blank lines
             $content =~ s#(^\s+|\s+$)##g;   # whitespace at the start/end of file
 
-            write_file($file, ";/* $files{$source} */\n" . $content . "\n");
+            write_text($file, ";/* $files{$source} */\n" . $content . "\n");
         }
         push @minified, $file;
     }
@@ -582,9 +581,9 @@ sub _concatenate_js {
     if (!-e $file) {
         my $content = '';
         foreach my $source (@minified) {
-            $content .= read_file($source);
+            $content .= read_text($source);
         }
-        write_file($file, $content);
+        write_text($file, $content);
     }
 
     $file =~ s/^\Q$cgi_path\E\///o;

Bugzilla/Util.pm

@@ -24,7 +24,7 @@ use parent qw(Exporter);
                              validate_email_syntax check_email_syntax clean_text
                              get_text template_var display_value disable_utf8
                              detect_encoding email_filter
-                             join_activity_entries);
+                             join_activity_entries read_text write_text);
 
 use Bugzilla::Constants;
 use Bugzilla::RNG qw(irand);
@@ -39,6 +39,8 @@ use Scalar::Util qw(tainted blessed);
 use Text::Wrap;
 use Encode qw(encode decode resolve_alias);
 use Encode::Guess;
+use File::Basename qw(dirname);
+use File::Temp qw(tempfile);
 
 sub trick_taint {
     require Carp;
@@ -106,6 +108,29 @@ sub html_quote {
     return $var;
 }
 
+sub read_text {
+    my ($filename) = @_;
+    open my $fh, '<:encoding(utf-8)', $filename;
+    local $/ = undef;
+    my $content = <$fh>;
+    close $fh;
+    return $content;
+}
+
+sub write_text {
+    my ($filename, $content) = @_;
+    my ($tmp_fh, $tmp_filename) = tempfile('.tmp.XXXXXXXXXX',
+        DIR    => dirname($filename),
+        UNLINK => 0,
+    );
+    binmode $tmp_fh, ':encoding(utf-8)';
+    print $tmp_fh $content;
+    close $tmp_fh;
+    # File::Temp tries for secure files, but File::Slurp used the umask.
+    chmod(0666 & ~umask, $tmp_filename);
+    rename $tmp_filename, $filename;
+}
+
 sub html_light_quote {
     my ($text) = @_;
     # admin/table.html.tmpl calls |FILTER html_light| many times.
@@ -588,7 +613,7 @@ sub datetime_from {
         second => defined($time[0]) ? int($time[0]) : undef,
         # If a timezone was specified, use it. Otherwise, use the
         # local timezone.
-        time_zone => Bugzilla->local_timezone->offset_as_string($time[6]) 
+        time_zone => DateTime::TimeZone->offset_as_string($time[6])
                      || Bugzilla->local_timezone,
     );