Bug 1079065: [SECURITY] Always use the 3 arguments form for open() to prevent shell...

r=dylan,a=simon

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 2fa9dfdbb8c54c0b480e67c2078489836f82b8d9..c58be60a4f1de8e03d5261727367e909fcfbbe75 100644 (file)
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -371,7 +371,7 @@ sub data {
     # If there's no attachment data in the database, the attachment is stored
     # in a local file, so retrieve it from there.
     if (length($self->{data}) == 0) {
-        if (open(AH, $self->_get_local_filename())) {
+        if (open(AH, '<', $self->_get_local_filename())) {
             local $/;
             binmode AH;
             $self->{data} = <AH>;
@@ -417,7 +417,7 @@ sub datasize {
     # is stored in a local file, and so retrieve its size from the file,
     # or the attachment has been deleted.
     unless ($self->{datasize}) {
-        if (open(AH, $self->_get_local_filename())) {
+        if (open(AH, '<', $self->_get_local_filename())) {
             binmode AH;
             $self->{datasize} = (stat(AH))[7];
             close(AH);

diff --git a/Bugzilla/Attachment/PatchReader.pm b/Bugzilla/Attachment/PatchReader.pm
index 01a624a8fa3f41cc704a3b20ecd297deb96362b2..4390c7055ec5840a55635725eda43828086b7443 100644 (file)
--- a/Bugzilla/Attachment/PatchReader.pm
+++ b/Bugzilla/Attachment/PatchReader.pm
@@ -110,7 +110,7 @@ sub process_interdiff {
     # Send through interdiff, send output directly to template.
     # Must hack path so that interdiff will work.
     $ENV{'PATH'} = $lc->{diffpath};
-    open my $interdiff_fh, "$lc->{interdiffbin} $old_filename $new_filename|";
+    open my $interdiff_fh, '-|', "$lc->{interdiffbin} $old_filename $new_filename";
     binmode $interdiff_fh;
     my ($reader, $last_reader) = setup_patch_readers("", $context);
 

diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm
index 00c699217b65c958deb0cebc73ef045ac3f1f1a3..385f1ce81ac0ebeeca9a5d0ba58bb42da278c623 100644 (file)
--- a/Bugzilla/Config/Common.pm
+++ b/Bugzilla/Config/Common.pm
@@ -256,7 +256,7 @@ sub check_webdotbase {
         # Check .htaccess allows access to generated images
         my $webdotdir = bz_locations()->{'webdotdir'};
         if(-e "$webdotdir/.htaccess") {
-            open HTACCESS, "$webdotdir/.htaccess";
+            open HTACCESS, "<", "$webdotdir/.htaccess";
             if(! grep(/ \\\.png\$/,<HTACCESS>)) {
                 return "Dependency graph images are not accessible.\nAssuming that you have not modified the file, delete $webdotdir/.htaccess and re-run checksetup.pl to rectify.\n";
             }

diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm
index 649fdd48666413c8efcfa06caff88fa68386faf5..c87b710ab9881abdf4c1bb632a7bbff4fd43ebde 100644 (file)
--- a/Bugzilla/Error.pm
+++ b/Bugzilla/Error.pm
@@ -92,7 +92,7 @@ sub _throw_error {
             $val = "*****" if $val =~ /password|http_pass/i;
             $mesg .= "[$$] " . Data::Dumper->Dump([$val],["env($var)"]);
         }
-        open(ERRORLOGFID, ">>$datadir/errorlog");
+        open(ERRORLOGFID, ">>", "$datadir/errorlog");
         print ERRORLOGFID "$mesg\n";
         close ERRORLOGFID;
     }

diff --git a/Bugzilla/Install/CPAN.pm b/Bugzilla/Install/CPAN.pm
index b1f3133c394a56e51a59e910ed3ab2179510567c..2ff3d8b101e9c0ae260dcb2efc6d20e0e05cb636 100644 (file)
--- a/Bugzilla/Install/CPAN.pm
+++ b/Bugzilla/Install/CPAN.pm
@@ -214,8 +214,8 @@ sub set_cpan_config {
     # Calling a senseless autoload that does nothing makes us
     # automatically load any existing configuration.
     # We want to avoid the "invalid command" message.
-    open(my $saveout, ">&STDOUT");
-    open(STDOUT, '>/dev/null');
+    open(my $saveout, ">&", "STDOUT");
+    open(STDOUT, '>', '/dev/null');
     eval { CPAN->ignore_this_error_message_from_bugzilla; };
     undef $@;
     close(STDOUT);

diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm
index 2881ab0478d66c2f25a4a1071cd4760506be0d8f..10fa5d6054673daca21d9314ef930a3806d19291 100644 (file)
--- a/Bugzilla/Install/Filesystem.pm
+++ b/Bugzilla/Install/Filesystem.pm
@@ -578,7 +578,7 @@ sub _update_old_charts {
                  ($in_file =~ /\.orig$/i));
 
         rename("$in_file", "$in_file.orig") or next;
-        open(IN, "$in_file.orig") or next;
+        open(IN, "<", "$in_file.orig") or next;
         open(OUT, '>', $in_file) or next;
 
         # Fields in the header

diff --git a/Bugzilla/Install/Localconfig.pm b/Bugzilla/Install/Localconfig.pm
index e15e235070cb77c62a5b0943d6a921d60cf8ea44..f56f8c02d27a4e5a3e041efb12e7d5985fee2efc 100644 (file)
--- a/Bugzilla/Install/Localconfig.pm
+++ b/Bugzilla/Install/Localconfig.pm
@@ -366,7 +366,7 @@ EOT
     # Move any custom or old variables into a separate file.
     if (scalar @old_vars) {
         my $filename_old = "$filename.old";
-        open(my $old_file, ">>$filename_old") || die "$filename_old: $!";
+        open(my $old_file, ">>", $filename_old) || die "$filename_old: $!";
         local $Data::Dumper::Purity = 1;
         foreach my $var (@old_vars) {
             print $old_file Data::Dumper->Dump([$localconfig->{$var}], 
@@ -383,7 +383,7 @@ EOT
     }
 
     # Re-write localconfig
-    open(my $fh, ">$filename") || die "$filename: $!";
+    open(my $fh, ">", $filename) || die "$filename: $!";
     foreach my $var (LOCALCONFIG_VARS) {
         print $fh "\n", $var->{desc},
                   Data::Dumper->Dump([$localconfig->{$var->{name}}],

diff --git a/collectstats.pl b/collectstats.pl
index e2af2f02eff54116d5dd355a523513aa095421b3..0c206698650aac7411463f28ac50e12233e59ce1 100755 (executable)
--- a/collectstats.pl
+++ b/collectstats.pl
@@ -349,7 +349,7 @@ sub regenerate_stats {
         return;
     }
 
-    if (open DATA, ">$file") {
+    if (open DATA, ">", $file) {
         my $fields = join('|', ('DATE', @statuses, @resolutions));
         print DATA <<FIN;
 # Bugzilla Daily Bug Stats

diff --git a/reports.cgi b/reports.cgi
index 2eacc61277e0002d54c3e810b66ab0b5cf6f83be..1f6c332ed1831242b20821428893b97fecec2d13 100755 (executable)
--- a/reports.cgi
+++ b/reports.cgi
@@ -177,7 +177,7 @@ sub generate_chart {
     $data_file =~ s/\//-/gs;
     $data_file = $dir . '/' . $data_file;
 
-    if (! open FILE, $data_file) {
+    if (!open(FILE, '<', $data_file)) {
         if ($product eq '-All-') {
             $product = '';
         }

diff --git a/search_plugin.cgi b/search_plugin.cgi
index 4dfe8fa9f4d3e3c5307e6934a10caf90e5bdc2c4..6040d02267f1c465711243c242b7784a1159e716 100755 (executable)
--- a/search_plugin.cgi
+++ b/search_plugin.cgi
@@ -33,7 +33,7 @@ print $cgi->header('application/xml');
 
 # Get the contents of favicon.ico
 my $filename = bz_locations()->{'libpath'} . "/images/favicon.ico";
-if (open(IN, $filename)) {
+if (open(IN, '<', $filename)) {
     local $/;
     binmode IN;
     $vars->{'favicon'} = <IN>;

diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi
index 162dd2afbbc2f7f856b02b3d6de37ea3c6e2cdec..cf4a907c0bfe04f76cbe86e39944bcc23f892b35 100755 (executable)
--- a/showdependencygraph.cgi
+++ b/showdependencygraph.cgi
@@ -61,7 +61,7 @@ sub CreateImagemap {
     my $map = "<map name=\"imagemap\">\n";
     my $default = "";
 
-    open MAP, "<$mapfilename";
+    open MAP, "<", $mapfilename;
     while(my $line = <MAP>) {
         if($line =~ /^default ([^ ]*)(.*)$/) {
             $default = qq{<area alt="" shape="default" href="$1">\n};
@@ -271,7 +271,7 @@ if ($webdotbase =~ /^https?:/) {
                                                  error => $! });
 
     binmode $pngfh;
-    open(DOT, "\"$webdotbase\" -Tpng $filename|");
+    open(DOT, '-|', "\"$webdotbase\" -Tpng $filename");
     binmode DOT;
     print $pngfh $_ while <DOT>;
     close DOT;
@@ -300,7 +300,7 @@ if ($webdotbase =~ /^https?:/) {
                                                  error => $! });
 
     binmode $mapfh;
-    open(DOT, "\"$webdotbase\" -Tismap $filename|");
+    open(DOT, '-|', "\"$webdotbase\" -Tismap $filename");
     binmode DOT;
     print $mapfh $_ while <DOT>;
     close DOT;

diff --git a/testserver.pl b/testserver.pl
index 3142685bc9b68646fdb0751fdb44cb2b30cdc1fe..ffe4b1ad5b3440a2945df358e548207bbd5c4aaf 100755 (executable)
--- a/testserver.pl
+++ b/testserver.pl
@@ -44,7 +44,7 @@ my @pscmds = ('ps -eo comm,gid', 'ps -acxo command,gid', 'ps -acxo command,rgid'
 my $sgid = 0;
 if (!ON_WINDOWS) {
     foreach my $pscmd (@pscmds) {
-        open PH, "$pscmd 2>/dev/null |";
+        open PH, '-|', "$pscmd 2>/dev/null";
         while (my $line = <PH>) {
             if ($line =~ /^(?:\S*\/)?(?:httpd|apache)2?\s+(\d+)$/) {
                 $sgid = $1 if $1 > $sgid;
@@ -271,7 +271,7 @@ sub check_image {
 
 sub create_file {
     my ($filename, $content) = @_;
-    open(FH, ">$filename")
+    open(FH, ">", $filename)
         or die "Failed to create $filename: $!\n";
     binmode FH;
     print FH $content;
@@ -280,7 +280,7 @@ sub create_file {
 
 sub read_file {
     my ($filename) = @_;
-    open(FH, $filename)
+    open(FH, "<", $filename)
         or die "Failed to open $filename: $!\n";
     binmode FH;
     my $content = <FH>;