Re: Can System() of Perl be bypassed?

From: Brian Hatch (secprogat_private)
Date: Thu Jan 23 2003 - 10:44:29 PST

  • Next message: Alex Russell: "Re: Standards for developing secure software"

    > Sandeep, the accepted way to avoid this problem is to use exec() instead of
    > system(), like so:
    
    If you need to send or receive output of your 'system' command,
    then indeed you want to use the fork/exec model so you can set
    up your input/output correctly (using pipes, usually.)
    
    If you only care about the exit status ($?), then the array form of
    the system command is sufficient.  It certainly takes up less space,
    and may be prefered for those afraid of doing things the long
    (but more C-like) way.
    
    It's always better to use something simpler that is as effective
    if you are not comfortable with the more detailed version.  Also
    keeps you from making mistakes.
    
    
    > #!/usr/bin/perl
    > 
    > ## Author: Ian Charnas <icc at cwru dot edu>
    > ## In this example, we pretend there is a web form with one
    > ## input field, named "searchstring".  This CGI would be the
    > ## 'action' for that form, and would simply grep through a file
    > ## (say, /usr/share/dict/words ) and return the matching lines.
    > 
    > ## Modules we'll need
    > use IO::Handle;
    > use CGI;
    > 
    > ## Setup CGI
    > $query = new CGI;
    > print $query->header('text/html');
    > 
    > my $pipereader = IO::Handle->new();
    > my $pipewriter = IO::Handle->new();
    > pipe($pipereader, $pipewriter);
    > 
    > if ($pid=fork()) {
    
    You should check for defined $pid  -- if the fork
    fails then $pid will be undefined.
    
    
    >     # this is the child, have it write to $pipewriter
    >     $pipereader->close();
    >     open(STDOUT, '>&' . $pipewriter->fileno());
    >     exec("/bin/grep", $query->param('searchstring'),
    > "/usr/share/dict/words");
    
    You should do some minimal sanity check on searchstring.
    What if they supply "-c"?  Then your grep line reads
    
    	grep -c /usr/share/dict/words
    
    and it will hang while searching STDIN for the string
    /usr/share/dict/words.  Can lead to an DOS condition
    if this CGI is called many times.  (Depending on
    your setup, STDIN may be empty by this point in
    which case it's a non-issue.)
    
    This is just a reminder that even if you know how to
    properly call your external command, you still need
    to sanitize the user input.
    
    Also, you need an 'exit(1)' after this exec line
    in case the exec fails for some reason.  (Say
    grep were not in /bin on this machine, or it were
    not executable.)
    
    > }
    
    The fact that there were mistakes in this code
    shows that sometimes the simpler way (system with
    list) is the better way.
    
    In the example CGI, you cannot use system with a list
    because you need it's output.  However you could use
    a similar shorter version: open2:
    
    	use IPC::Open2;
    
    	pipe(READ,WRITE) or die;
    
    	open2(\*READ, \*WRITE, '/bin/grep', $sanitized_arg,
    		"/usr/dict/words") or die;
    
    	while (<READ>) {
    		...
    	}
    
    Using built in functions would have saved the problems
    in the previous code listing, and make the program much
    easier to read as well.
    
    (Note: open2 can take a string or a list, just like
    system, so use the list form.)
    
    
    --
    Brian Hatch                  "UNIX was not designed to
       Systems and               stop you from doing stupid
       Security Engineer         things, because that would
    www.hackinglinuxexposed.com  also stop you from doing
                                 clever things."
    Every message PGP signed
    
    
    



    This archive was generated by hypermail 2b30 : Thu Jan 23 2003 - 12:49:42 PST