From 7251cc2c23e5392b43d6fb56d0ec3c3092f1ffe8 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Wed, 11 Aug 2010 17:38:15 -0700
Subject: [PATCH] Secure and fix the Zip Archive.

---
 docs/changelog/7.x.x.txt            |  1 +
 lib/WebGUI/Asset/File/ZipArchive.pm | 32 ++++++++++++++++++---
 t/Asset/File/ZipArchive.t           | 44 +++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 t/Asset/File/ZipArchive.t

diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt
index b82b90e51..632f6c41b 100644
--- a/docs/changelog/7.x.x.txt
+++ b/docs/changelog/7.x.x.txt
@@ -5,6 +5,7 @@
  - fixed possible vulnerability loading help files
  - fixed possible vulnerability with filenames in storage locations
  - fixed possible vulnerability with extracting tar files in storage locations
+ - fixed possible vulnerability with extracting files in Zip Archive assets.
 
 7.9.11
  - fixed #11755: New cart does not update shipping methods correctly
diff --git a/lib/WebGUI/Asset/File/ZipArchive.pm b/lib/WebGUI/Asset/File/ZipArchive.pm
index d4c5454d1..bbe0f2b96 100644
--- a/lib/WebGUI/Asset/File/ZipArchive.pm
+++ b/lib/WebGUI/Asset/File/ZipArchive.pm
@@ -74,19 +74,21 @@ sub unzip {
     my $dir_guard = Scope::Guard->new(sub { chdir $cwd });
    
 	my $i18n = WebGUI::International->new($self->session,"Asset_ZipArchive");
-	if ($filename =~ m/\.zip/i) {
+	if ($filename =~ m/\.zip$/i) {
 		my $zip = Archive::Zip->new();
 		unless ($zip->read($filename) == $zip->AZ_OK){
 			$self->session->errorHandler->warn($i18n->get("zip_error"));
 			return 0;
 		}
-		$zip->extractTree();  
-	} elsif ($filename =~ m/\.tar/i) {
+		$zip->extractTree();
+        $self->fixFilenames;
+	} elsif ($filename =~ m/\.tar$/i) {
 		Archive::Tar->extract_archive($filepath.'/'.$filename,1);
 		if (Archive::Tar->error) {
 			$self->session->errorHandler->warn(Archive::Tar->error);
 			return 0;
 		}
+        $self->fixFilenames;
 	} else {
 		$self->session->errorHandler->warn($i18n->get("bad_archive"));
 	}
@@ -153,6 +155,28 @@ sub definition {
 }
 
 
+#-------------------------------------------------------------------
+
+=head2 fixFilenames ( )
+
+Fix any files with dangerous extensions, in all files that were extracted.  This is done
+locally, because if we used a method from Storage, then it would also rename HTML files.
+
+=cut
+
+sub fixFilenames {
+	my $self    = shift;
+    my $storage = $self->getStorageLocation;
+    my $files   = $storage->getFiles('all');
+    FILE: foreach my $file (@{ $files }) {
+        my $extension = $storage->getFileExtension($file);
+        next FILE unless isIn($extension, qw/pl perl pm cgi php asp sh/);
+        my $newFile = $file;
+        $newFile =~ s/\.$extension/_$extension.txt/;
+        $storage->renameFile($file, $newFile);
+    }
+}
+
 #-------------------------------------------------------------------
 
 =head2 prepareView ( )
@@ -196,7 +220,7 @@ sub processPropertiesFromFormPost {
 		return undef;
 	}
 	
-	unless ($file =~ m/\.tar/i || $file =~ m/\.zip/i) {
+	unless ($file =~ m/\.tar$/i || $file =~ m/\.zip$/i) {
 		$storage->delete;
 		$self->session->db->write("update FileAsset set filename=NULL where assetId=".$self->session->db->quote($self->getId));
 		$self->session->scratch->set("za_error",$i18n->get("za_error"));
diff --git a/t/Asset/File/ZipArchive.t b/t/Asset/File/ZipArchive.t
new file mode 100644
index 000000000..5f4e615bf
--- /dev/null
+++ b/t/Asset/File/ZipArchive.t
@@ -0,0 +1,44 @@
+#-------------------------------------------------------------------
+# WebGUI is Copyright 2001-2009 Plain Black Corporation.
+#-------------------------------------------------------------------
+# Please read the legal notices (docs/legal.txt) and the license
+# (docs/license.txt) that came with this distribution before using
+# this software.
+#-------------------------------------------------------------------
+# http://www.plainblack.com                     info@plainblack.com
+#-------------------------------------------------------------------
+
+use FindBin;
+use strict;
+use lib "$FindBin::Bin/../../lib";
+
+use WebGUI::Storage;
+use WebGUI::Asset;
+use WebGUI::Asset::File::ZipArchive;
+
+use WebGUI::Test;
+use Test::More; # increment this value for each test you create
+use Test::Deep;
+plan tests => 2;
+
+my $session = WebGUI::Test->session;
+
+my $node = WebGUI::Asset->getImportNode($session);
+
+my $arch = $node->addChild({
+    className => 'WebGUI::Asset::File::ZipArchive',
+});
+
+WebGUI::Test->addToCleanup($arch);
+
+my $storage = $arch->getStorageLocation;
+$storage->addFileFromFilesystem(WebGUI::Test->getTestCollateralPath('extensions.tar'));
+ok($arch->unzip($storage, 'extensions.tar'), 'unzip returns true when it successfully unpacked');
+
+$arch->fixFilenames();
+
+cmp_bag(
+    $storage->getFiles, 
+    [ qw{ extensions.tar extension_pm.txt extension_perl.txt extension.html extensions extensions/extension.html }], 
+    'files after fixFilenames, html files left alone'
+);