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 c04227215dafe8147fa70cda53580f03684778ec..f670425999fb4b9edc10a3871d32e7abb8cb38cf 100755 (executable)
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -873,14 +873,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 246342e580d1f8826d9feea7ccbfc1ebd3a23213..ed1830581ac2b67287bfa71dd75420f8b44c73ce 100644 (file)
--- a/defparams.pl
+++ b/defparams.pl
@@ -99,7 +99,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>)) {
                 return "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 38833ce153f50ac6be337ec4e3e50177d9f971d9..805bba882a5a65300b2319992348969f6c7b33b3 100644 (file)
--- a/globals.pl
+++ b/globals.pl
@@ -237,19 +237,21 @@ 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";
 
-    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";
+    require File::Temp;
+    my ($fh, $tmpname) = File::Temp::tempfile("versioncache.XXXXX",
+                                              DIR => "data");
+
+    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";
 
     require Data::Dumper;
-    print FID Data::Dumper->Dump([\@::log_columns, \%::versions],
-                                 ['*::log_columns', '*::versions']);
+    print($fh,Data::Dumper->Dump([\@::log_columns, \%::versions],
+                                 ['*::log_columns', '*::versions']));
 
     foreach my $i (@list) {
         if (!defined $::components{$i}) {
@@ -257,23 +259,23 @@ sub GenerateVersionTable {
         }
     }
     @::legal_versions = sort {uc($a) cmp uc($b)} keys(%varray);
-    print FID Data::Dumper->Dump([\@::legal_versions, \%::components],
-                                 ['*::legal_versions', '*::components']);
+    print($fh,Data::Dumper->Dump([\@::legal_versions, \%::components],
+                                 ['*::legal_versions', '*::components']));
     @::legal_components = sort {uc($a) cmp uc($b)} keys(%carray);
 
-    print FID Data::Dumper->Dump([\@::legal_components, \@::legal_product,
+    print($fh,Data::Dumper->Dump([\@::legal_components, \@::legal_product,
                                   \@::legal_priority, \@::legal_severity,
                                   \@::legal_platform, \@::legal_opsys,
                                   \@::legal_bug_status, \@::legal_resolution],
                                  ['*::legal_components', '*::legal_product',
                                   '*::legal_priority', '*::legal_severity',
                                   '*::legal_platform', '*::legal_opsys',
-                                  '*::legal_bug_status', '*::legal_resolution']);
+                                  '*::legal_bug_status', '*::legal_resolution']));
 
-    print FID Data::Dumper->Dump([\@::settable_resolution, \%::proddesc,
+    print($fh,Data::Dumper->Dump([\@::settable_resolution, \%::proddesc,
                                   \@::enterable_products, \%::prodmaxvotes],
                                  ['*::settable_resolution', '*::proddesc',
-                                  '*::enterable_products', '*::prodmaxvotes']);
+                                  '*::enterable_products', '*::prodmaxvotes']));
 
     if ($dotargetmilestone) {
         # reading target milestones in from the database - matthew@zeroknowledge.com
@@ -296,12 +298,12 @@ sub GenerateVersionTable {
             }
         }
 
-        print FID Data::Dumper->Dump([\%::target_milestone,
+        print($fh,Data::Dumper->Dump([\%::target_milestone,
                                       \@::legal_target_milestone,
                                       \%::milestoneurl],
                                      ['*::target_milestone',
                                       '*::legal_target_milestone',
-                                      '*::milestoneurl']);
+                                      '*::milestoneurl']));
     }
 
     SendSQL("SELECT id, name FROM keyworddefs ORDER BY name");
@@ -312,11 +314,11 @@ sub GenerateVersionTable {
         $::keywordsbyname{$name} = $id;
     }
 
-    print FID Data::Dumper->Dump([\@::legal_keywords, \%::keywordsbyname],
-                                 ['*::legal_keywords', '*::keywordsbyname']);
+    print($fh,Data::Dumper->Dump([\@::legal_keywords, \%::keywordsbyname],
+                                 ['*::legal_keywords', '*::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 37b7c74cc4d44b352ac80fdb63f61bb9e3b147fc..61278b5f32b7de8ab652a1771c79355525e15637 100755 (executable)
--- a/showdependencygraph.cgi
+++ b/showdependencygraph.cgi
@@ -25,6 +25,8 @@ use strict;
 
 use lib qw(.);
 
+use File::Temp;
+
 require "CGI.pl";
 
 ConnectToDatabase();
@@ -62,11 +64,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;
     }
@@ -78,12 +80,13 @@ if (!defined($::FORM{'id'}) && !defined($::FORM{'doall'})) {
     ThrowCodeError("missing_bug_id");
 }    
 
-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]
 };
@@ -95,7 +98,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'})) {
@@ -119,7 +122,7 @@ if ($::FORM{'doall'}) {
                 push @stack, $dependson;
             }
 
-            AddLink($blocked, $dependson);
+            AddLink($blocked, $dependson, $fh);
         }
     }
 
@@ -159,15 +162,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;
 
@@ -180,11 +183,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);
 }