block the same set of extensions in http, scalar and file uploads in Storage.

docs/changelog/7.x.x.txt

@@ -3,6 +3,7 @@
  - fixed #11779: SQLReport can run arbitrary queries
  - fixed possible vulnerability loading template parser
  - fixed possible vulnerability loading help files
+ - fixed possible vulnerability with filenames in storage locations
 
 7.9.11
  - fixed #11755: New cart does not update shipping methods correctly

lib/WebGUI/Storage.pm

@@ -315,10 +315,7 @@ sub addFileFromFilesystem {
         return undef;
     }
     my $filename = (File::Spec->splitpath( $pathToFile ))[2];
-    if (isIn($self->getFileExtension($filename), qw(pl perl sh cgi php asp pm))) {
+    $filename = $self->block_extensions($filename);
-        $filename =~ s/\./\_/g;
-        $filename .= ".txt";
-    }
     $filename = $self->session->url->makeCompliant($filename);
     my $source;
     my $dest;
@@ -383,11 +380,7 @@ sub addFileFromFormPost {
             if ($upload->size > 1024 * $self->session->setting->get("maxAttachmentSize"));
         $clientFilename =~ s/.*[\/\\]//;
         $clientFilename =~ s/^thumb-//;
-        my $type = $self->getFileExtension($clientFilename);
+        $clientFilename = $self->block_extensions($clientFilename);
-        if (isIn($type, qw(pl perl sh cgi php asp html htm))) { # make us safe from malicious uploads
-            $clientFilename =~ s/\./\_/g;
-            $clientFilename .= ".txt";
-        }
         $filename = $session->url->makeCompliant($clientFilename);
         my $filePath = $self->getPath($filename);
         $attachmentCount++;
@@ -452,10 +445,7 @@ The content to write to the file.
 
 sub addFileFromScalar {
 	my ($self, $filename, $content) = @_;
-    if (isIn($self->getFileExtension($filename), qw(pl perl sh cgi php asp html htm))) { # make us safe from malicious uploads
+    $filename = $self->block_extensions($filename);
-        $filename =~ s/\./\_/g;
-        $filename .= ".txt";
-    }
     $filename = $self->session->url->makeCompliant($filename);
 	if (open(my $FILE, ">", $self->getPath($filename))) {
 		print $FILE $content;
@@ -501,6 +491,32 @@ sub adjustMaxImageSize {
 
 #-------------------------------------------------------------------
 
+=head2 block_extensions ( $file )
+
+Rename files so they can't be used for malicious purposes.  The list of bad extensions
+includs shell script, perl scripts, php, ASP, perl modules and HTML files.
+
+Any file found with a bad extension will be renamed from file.ext to file_ext.txt
+
+=head3 $file
+
+The file to check for bad extensions.
+
+=cut 
+
+sub block_extensions {
+    my $self = shift;
+    my $file = shift;
+    my $extension = $self->getFileExtension($file);
+    if (isIn($extension, qw(pl perl sh cgi php asp pm html htm))) {
+        $file =~ s/\.$extension/\_$extension/;
+        $file .= ".txt";
+    }
+    return $file;
+}
+
+#-------------------------------------------------------------------
+
 =head2 clear ( )
 
 Clears a storage location of all files.  If configured for CDN, add

t/Storage.t

@@ -29,10 +29,14 @@ my $session = WebGUI::Test->session;
 
 my $cwd = Cwd::cwd();
 
-my ($extensionTests, $fileIconTests) = setupDataDrivenTests($session);
+my ($extensionTests, $fileIconTests, $block_extension_tests) = setupDataDrivenTests($session);
 
 my $numTests = 140; # increment this value for each test you create
-plan tests => $numTests + scalar @{ $extensionTests } + scalar @{ $fileIconTests };
+plan tests => 140
+            + scalar @{ $extensionTests }
+            + scalar @{ $fileIconTests  }
+            + scalar @{ $block_extension_tests }
+            ;
 
 my $uploadDir = $session->config->get('uploadsPath');
 ok ($uploadDir, "uploadDir defined in config");
@@ -375,6 +379,17 @@ ok (-e $tempStor->getPath(), '... directory was created');
 ok($tempStor->getHexId, '... getHexId returns something');
 is($tempStor->getHexId, $session->id->toHex($tempStor->getId), '... returns the hexadecimal value of the GUID');
 
+####################################################
+#
+# block_extensions
+#
+####################################################
+
+##Run a set of extensions through and watch how the files get changed.
+foreach my $extTest (@{ $block_extension_tests }) {
+	is( $storage1->block_extensions($extTest->{filename}), $extTest->{blockname}, $extTest->{comment} );
+}
+
 ####################################################
 #
 # tar
@@ -472,17 +487,17 @@ is($formStore->addFileFromFormPost('files'), undef, 'addFileFromFormPost returns
 
 $session->request->uploadFiles(
     'oneFile',
-    [ WebGUI::Test->getTestCollateralPath('WebGUI.pm') ],
+    [ WebGUI::Test->getTestCollateralPath('littleTextFile') ],
 );
-is($formStore->addFileFromFormPost('oneFile'), 'WebGUI.pm', '... returns the name of the uploaded file');
+is($formStore->addFileFromFormPost('oneFile'), 'littleTextFile', '... returns the name of the uploaded file');
-cmp_bag($formStore->getFiles, [ qw/WebGUI.pm/ ], '... adds the file to the storage location');
+cmp_bag($formStore->getFiles, [ qw/littleTextFile/ ], '... adds the file to the storage location');
 
 $session->request->uploadFiles(
     'thumbFile',
     [ WebGUI::Test->getTestCollateralPath('thumb-thumb.gif') ],
 );
 is($formStore->addFileFromFormPost('thumbFile'), 'thumb.gif', '... strips thumb- prefix from files');
-cmp_bag($formStore->getFiles, [ qw/WebGUI.pm thumb.gif/ ], '... adds the file to the storage location');
+cmp_bag($formStore->getFiles, [ qw/littleTextFile thumb.gif/ ], '... adds the file to the storage location');
 
 ####################################################
 #
@@ -748,6 +763,63 @@ sub setupDataDrivenTests {
         },
     ];
 
+    my $block_extension_tests = [
+        {
+            filename  => 'filename',
+            blockname => 'filename',
+            comment  => 'no extension',
+        },
+        {
+            filename  => 'filename.pl',
+            blockname => 'filename_pl.txt',
+            comment  => 'pl file',
+        },
+        {
+            filename  => 'filename.perl',
+            blockname => 'filename_perl.txt',
+            comment  => 'perl file',
+        },
+        {
+            filename  => 'filename.cgi',
+            blockname => 'filename_cgi.txt',
+            comment  => 'cgi file',
+        },
+        {
+            filename  => 'filename.php',
+            blockname => 'filename_php.txt',
+            comment  => 'php file',
+        },
+        {
+            filename  => 'filename.asp',
+            blockname => 'filename_asp.txt',
+            comment  => 'asp file',
+        },
+        {
+            filename  => 'filename.pm',
+            blockname => 'filename_pm.txt',
+            comment  => 'perl module file',
+        },
+        {
+            filename  => 'filename.htm',
+            blockname => 'filename_htm.txt',
+            comment  => 'htm file',
+        },
+        {
+            filename  => 'filename.html',
+            blockname => 'filename_html.txt',
+            comment  => 'html file',
+        },
+        {
+            filename  => 'filename.pm.txt',
+            blockname => 'filename.pm.txt',
+            comment  => 'internal .pm not touched',
+        },
+        {
+            filename  => 'filename.txt.pm',
+            blockname => 'filename.txt_pm.txt',
+            comment  => 'double extension handled',
+        },
+    ];
     my $fileIconTests = [
         {
             filename => 'filename',
@@ -771,5 +843,5 @@ sub setupDataDrivenTests {
         },
     ];
 
-    return ($extensionTests, $fileIconTests)
+    return ($extensionTests, $fileIconTests, $block_extension_tests);
 }