Re: cleanzip

[graff]

I think you'll want to be careful about strings that get passed to sub-shells, whether via system() calls or via back-ticks. At best, the job can just fail because of a file name that happens to contain a space or other shell meta-character (and at worst, you might be unpacking zip files whose contents include file names like foo; rm -rf /*).

You can easily change your one system call to the list style:

system( 'unzip', $ZIP, '-d', $TMP );  # instead of 'system "unzip $ZIP
+ -d $TMP"'

For the back-ticks, where you want to the command's stdout to be assigned to a variable, it might suffice to put backslash in front of any shell-magic characters:

$ZIP =~ s/([^\w.-])/\\$1/g;
for $BADFILE ( @BADFILES ) {
   if ($BADFILE=~s/:.*FOUND$//) {
      $BADFILE=substr($BADFILE,length($TMP)+1);
      $BADFILE =~ s/([^\w.-])/\\$1/g;
      $RESULT = `zip -d $ZIP $BADFILE`;
      print $RESULT;
   } ...

(not tested)

Bear in mind that in the OP code, the placement of double-quotes around "$BADFILE" in the back-tick command (running "zip -d ...") will do no good when the file name happens to contain one or more double-quote characters.

[sflitman]

Agree fully. I've also posted execute() which uses an exec-like mechanism, bypassing shell issues for arguments. Here it is again:

sub execute {       # execute a command without shell but with timeout
   my ($cmd,@args)=@_;
   my $timeout=15;  # seconds
   my ($result,$pid,$i,$time);
   if ($args[$#args]=~/^--?timeout=(\d+)$/i) {  # or pass as last arg
      $timeout=$1;
      pop @args;
   }
   die "execute($cmd,".join(',',@args)."): null arguments"    # SSF 08
+0708 eliminate null arguments, they are always erroneous
      if grep { length($_)==0 } @args;  
   $i=index($cmd,' ');   # args appended to command?
   if ($i>-1) {
      unshift @args,split(/\s+/,substr($cmd,$i+1));   # could be more 
+than one
      $cmd=substr($cmd,0,$i);
   }
   eval {
    local $SIG{ALRM} = sub { die "alarm\n" }; # NB: \n required
        local($/)=undef;
    alarm $timeout;
        $time=time if debug_has(EXEC);
        $pid=open(CMD,'-|',$cmd,@args);   # run without shell overhead
        if ($pid) {
           $result=<CMD>;
           close CMD;
           debug sprintf("execute(%s%s%s):\n%s%s [%.3f s]",$cmd,@args?
+',':'',join(',',@args),substr($result,0,500),length($result)>500?'...
+':'',time-$time) if debug_has(EXEC);
        } else {
           alarm 0;
           die "execute($cmd ".join(' ',@args)."): $!" unless $pid;
        }     
    alarm 0;
    };
    if ($@) {
    die $@ unless $@ eq "alarm\n";   # propagate unexpected errors
    }
    $result;
}

SSF