check for zip and unzip commands, use multi-arg version of system to prevent the shell getting its hands on things (metacharacters in pw or whatnot), RT#5650

2 files changed, 9 insertions, 3 deletions

diff --git a/FS/bin/freeside-paymentech-download b/FS/bin/freeside-paymentech-download
index 8fb4bcc..f4e0391 100755
--- a/FS/bin/freeside-paymentech-download
+++ b/FS/bin/freeside-paymentech-download
@@ -30,6 +30,8 @@ if ( $opt_a ) {
     unless -w $opt_a;
 }
 
+my $unzip_check = `which unzip` or die "can't find unzip executable\n";
+
 #my $tmpdir = File::Temp->newdir();
 my $tmpdir = tempdir( CLEANUP => 1 ); #DIR=>somewhere?
 
@@ -64,9 +66,10 @@ BATCH: foreach my $filename (@files) {
   }
 
   #unzip file
-  system("unzip -P $password -q $tmpdir/${filename}_resp.zip -d $tmpdir");
+  system('unzip', '-P', '$password', '-q',
+           "$tmpdir/${filename}_resp.zip", '-d', $tmpdir);
   if(! -f "$tmpdir/${filename}_resp.xml") {
-    warn "failed to extract ${filename}_resp.xml\n";
+    warn "failed to extract ${filename}_resp.xml from ${filename}_resp.zip\n";
     next BATCH;
   }
 
diff --git a/FS/bin/freeside-paymentech-upload b/FS/bin/freeside-paymentech-upload
index 08a59c5..985095e 100755
--- a/FS/bin/freeside-paymentech-upload
+++ b/FS/bin/freeside-paymentech-upload
@@ -25,6 +25,8 @@ sub usage { "
 my $user = shift or die &usage;
 adminsuidsetup $user;
 
+my $zip_check = `which zip` or die "can't find zip executable\n";
+
 my @batches; 
 
 if($opt_a) {
@@ -60,7 +62,8 @@ foreach my $pay_batch (@batches) {
   print OUT $text;
   close OUT;
 
-  system("zip -P $password -q -j $tmpdir/$filename.zip $tmpdir/$filename.xml");
+  system('zip', '-P', '$password', '-q', '-j',
+           "$tmpdir/$filename.zip", "$tmpdir/$filename.xml");
 
   die "failed to create zip file\n" if (! -f "$tmpdir/$filename.zip" );
   push @filenames, $filename;