Bug 197153: Fix for insecure temporary filename handling.

Patch by Brad Baetz <bbaetz@acm.org>
r= justdave, gerv
a= justdave

diff --git a/checksetup.pl b/checksetup.pl
index d86c12b2a2070966ec30367997e4c147bfb2de30..b23ca76a6bb813bb73d0bf7c17cdbc8629560e0f 100755 (executable)
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -203,6 +203,10 @@ my $modules = [
         name => 'File::Spec', 
         version => '0.82' 
     }, 
+    { 
+        name => 'File::Temp',
+        version => '0'
+    },
     { 
         name => 'Template', 
         version => '2.07' 
@@ -813,14 +817,13 @@ END
 # Restrict access to .dot files to the public webdot server at research.att.com 
 # if research.att.com ever changed their IP, or if you use a different
 # webdot server, you'll need to edit this
-<FilesMatch ^[0-9]+\.dot$>
+<FilesMatch \.dot$>
   Allow from 192.20.225.10
   Deny from all
 </FilesMatch>
 
-# Allow access by a local copy of 'dot' to .png, .gif, .jpg, and
-# .map files
-<FilesMatch ^[0-9]+\.(png|gif|jpg|map)$>
+# Allow access to .png files created by a local copy of 'dot'
+<FilesMatch \.png$>
   Allow from all
 </FilesMatch>
 

diff --git a/defparams.pl b/defparams.pl
index d5c41dc601f820419d53424ef68c1cc9239fc3c0..973e4a73f482005c2a09494add7a0af13b4dc4f3 100644 (file)
--- a/defparams.pl
+++ b/defparams.pl
@@ -55,15 +55,16 @@ sub WriteParams {
         }
     }
 
-    my $tmpname = "data/params.$$";
-    open(FID, ">$tmpname") || die "Can't create $tmpname";
+    require File::Temp;
+    my ($fh, $tmpname) = File::Temp::tempfile("params.XXXXX",
+                                              DIR=>'data');
     my $v = $::param{'version'};
     delete $::param{'version'};  # Don't write the version number out to
                                 # the params file.
-    print FID GenerateCode('%::param');
+    print $fh GenerateCode('%::param');
     $::param{'version'} = $v;
-    print FID "1;\n";
-    close FID;
+    print $fh "1;\n";
+    close $fh;
     rename $tmpname, "data/params" || die "Can't rename $tmpname to data/params";
     ChmodDataFile('data/params', 0666);
 }
@@ -439,7 +440,7 @@ sub check_webdotbase {
         # Check .htaccess allows access to generated images
         if(-e "data/webdot/.htaccess") {
             open HTACCESS, "data/webdot/.htaccess";
-            if(! grep(/png/,<HTACCESS>)) {
+            if(! grep(/ \\\.png\$/,<HTACCESS>)) {
                 print "Dependency graph images are not accessible.\nDelete data/webdot/.htaccess and re-run checksetup.pl to rectify.\n";
             }
             close HTACCESS;

diff --git a/globals.pl b/globals.pl
index c36ffd51e45886f9c84516b64490c5d116f55a69..a00df03d2eaf73bf347dcf4714e9410659056c98 100644 (file)
--- a/globals.pl
+++ b/globals.pl
@@ -517,18 +517,19 @@ sub GenerateVersionTable {
 
     my @list = sort { uc($a) cmp uc($b)} keys(%::versions);
     @::legal_product = @list;
-    my $tmpname = "data/versioncache.$$";
-    open(FID, ">$tmpname") || die "Can't create $tmpname";
+    require File::Temp;
+    my ($fh, $tmpname) = File::Temp::tempfile("versioncache.XXXXX",
+                                              DIR=>'data');
 
-    print FID "#\n";
-    print FID "# DO NOT EDIT!\n";
-    print FID "# This file is automatically generated at least once every\n";
-    print FID "# hour by the GenerateVersionTable() sub in globals.pl.\n";
-    print FID "# Any changes you make will be overwritten.\n";
-    print FID "#\n";
+    print $fh "#\n";
+    print $fh "# DO NOT EDIT!\n";
+    print $fh "# This file is automatically generated at least once every\n";
+    print $fh "# hour by the GenerateVersionTable() sub in globals.pl.\n";
+    print $fh "# Any changes you make will be overwritten.\n";
+    print $fh "#\n";
 
-    print FID GenerateCode('@::log_columns');
-    print FID GenerateCode('%::versions');
+    print $fh GenerateCode('@::log_columns');
+    print $fh GenerateCode('%::versions');
 
     foreach my $i (@list) {
         if (!defined $::components{$i}) {
@@ -536,18 +537,18 @@ sub GenerateVersionTable {
         }
     }
     @::legal_versions = sort {uc($a) cmp uc($b)} keys(%varray);
-    print FID GenerateCode('@::legal_versions');
-    print FID GenerateCode('%::components');
+    print $fh GenerateCode('@::legal_versions');
+    print $fh GenerateCode('%::components');
     @::legal_components = sort {uc($a) cmp uc($b)} keys(%carray);
-    print FID GenerateCode('@::legal_components');
+    print $fh GenerateCode('@::legal_components');
     foreach my $i('product', 'priority', 'severity', 'platform', 'opsys',
                   'bug_status', 'resolution') {
-        print FID GenerateCode('@::legal_' . $i);
+        print $fh GenerateCode('@::legal_' . $i);
     }
-    print FID GenerateCode('@::settable_resolution');
-    print FID GenerateCode('%::proddesc');
-    print FID GenerateCode('@::enterable_products');
-    print FID GenerateCode('%::prodmaxvotes');
+    print $fh GenerateCode('@::settable_resolution');
+    print $fh GenerateCode('%::proddesc');
+    print $fh GenerateCode('@::enterable_products');
+    print $fh GenerateCode('%::prodmaxvotes');
 
     if ($dotargetmilestone) {
         # reading target milestones in from the database - matthew@zeroknowledge.com
@@ -567,9 +568,9 @@ sub GenerateVersionTable {
             }
         }
 
-        print FID GenerateCode('%::target_milestone');
-        print FID GenerateCode('@::legal_target_milestone');
-        print FID GenerateCode('%::milestoneurl');
+        print $fh GenerateCode('%::target_milestone');
+        print $fh GenerateCode('@::legal_target_milestone');
+        print $fh GenerateCode('%::milestoneurl');
     }
 
     SendSQL("SELECT id, name FROM keyworddefs ORDER BY name");
@@ -579,11 +580,11 @@ sub GenerateVersionTable {
         $name = lc($name);
         $::keywordsbyname{$name} = $id;
     }
-    print FID GenerateCode('@::legal_keywords');
-    print FID GenerateCode('%::keywordsbyname');
+    print $fh GenerateCode('@::legal_keywords');
+    print $fh GenerateCode('%::keywordsbyname');
 
-    print FID "1;\n";
-    close FID;
+    print $fh "1;\n";
+    close $fh;
     rename $tmpname, "data/versioncache" || die "Can't rename $tmpname to versioncache";
     ChmodDataFile('data/versioncache', 0666);
 }

diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi
index 40245060c7dd7ada4f6b5da997004e19543e621e..57098c783801727d562bdc3771de24c50af26c99 100755 (executable)
--- a/showdependencygraph.cgi
+++ b/showdependencygraph.cgi
@@ -26,6 +26,8 @@ use strict;
 
 use lib qw(.);
 
+use File::Temp;
+
 require "CGI.pl";
 
 ConnectToDatabase();
@@ -59,11 +61,11 @@ sub CreateImagemap {
 }
 
 sub AddLink {
-    my ($blocked, $dependson) = (@_);
+    my ($blocked, $dependson, $fh) = (@_);
     my $key = "$blocked,$dependson";
     if (!exists $edgesdone{$key}) {
         $edgesdone{$key} = 1;
-        print DOT "$blocked -> $dependson\n";
+        print $fh "$blocked -> $dependson\n";
         $seen{$blocked} = 1;
         $seen{$dependson} = 1;
     }
@@ -76,12 +78,13 @@ if (!defined($::FORM{'id'}) && !defined($::FORM{'doall'})) {
     exit;
 }    
 
-my $filename = "data/webdot/$$.dot";
+my ($fh, $filename) = File::Temp::tempfile("XXXXXXXXXX",
+                                           SUFFIX => '.dot',
+                                           DIR => "data/webdot");
 my $urlbase = Param('urlbase');
 
-open(DOT, ">$filename") || die "Can't create $filename";
-print DOT "digraph G {";
-print DOT qq{
+print $fh "digraph G {";
+print $fh qq{
 graph [URL="${urlbase}query.cgi", rankdir=$::FORM{'rankdir'}, size="64,64"]
 node [URL="${urlbase}show_bug.cgi?id=\\N", style=filled, color=lightgrey]
 };
@@ -93,7 +96,7 @@ if ($::FORM{'doall'}) {
 
     while (MoreSQLData()) {
         my ($blocked, $dependson) = FetchSQLData();
-        AddLink($blocked, $dependson);
+        AddLink($blocked, $dependson, $fh);
     }
 } else {
     foreach my $i (split('[\s,]+', $::FORM{'id'})) {
@@ -117,7 +120,7 @@ if ($::FORM{'doall'}) {
                 push @stack, $dependson;
             }
 
-            AddLink($blocked, $dependson);
+            AddLink($blocked, $dependson, $fh);
         }
     }
 
@@ -157,15 +160,15 @@ foreach my $k (keys(%seen)) {
     }
 
     if (@params) {
-        print DOT "$k [" . join(',', @params) . "]\n";
+        print $fh "$k [" . join(',', @params) . "]\n";
     } else {
-        print DOT "$k\n";
+        print $fh "$k\n";
     }
 }
 
 
-print DOT "}\n";
-close DOT;
+print $fh "}\n";
+close $fh;
 
 chmod 0777, $filename;
 
@@ -178,11 +181,23 @@ if ($webdotbase =~ /^https?:/) {
      $vars->{'map_url'} = $url . ".map";
 } else {
     # Local dot installation
-    my $pngfilename = "data/webdot/$$.png";
-    my $mapfilename = "data/webdot/$$.map";
-    system("$webdotbase","-Tpng","-o","$pngfilename","$filename");
+    my $dotfh;
+    my ($pngfh, $pngfilename) = File::Temp::tempfile("XXXXXXXXXX",
+                                                     SUFFIX => '.png',
+                                                     DIR => 'data/webdot');
+    open (DOT, '-|') or exec ($webdotbase, "-Tpng", $filename);
+    print $pngfh $_ while <DOT>;
+    close DOT;
+    close $pngfh;
     $vars->{'image_url'} = $pngfilename;
-    system("$webdotbase","-Tismap","-o","$mapfilename","$filename");
+
+    my ($mapfh, $mapfilename) = File::Temp::tempfile("XXXXXXXXXX",
+                                                     SUFFIX => '.map',
+                                                     DIR => 'data/webdot');
+    open (DOT, '-|') or exec ($webdotbase, "-Tismap", $filename);
+    print $mapfh $_ while <DOT>;
+    close DOT;
+    close $mapfh;
     $vars->{'image_map'} = CreateImagemap($mapfilename);
 }