comment on

This is more secure shell programming than secure perl programming, per se, but when passing arguments to an external command, in addition to the advice above about general control-character cleaning and proper escaping, be wary of cases where the passed argument might be interpreted as an option. For example, consider this code that might be part of a man2html gateway:

# $page and $section are parameters from the user that have been clean
+ed of 0 bytes and obvious control characters
  my $mantext = ''; my $status;
  my $pid = open(KID_STDOUT, "-|");
  if (not defined $pid) {
    die "cannot fork: $!; bailing out";
  }
  if ($pid) { ## parent
    while(<KID_STDOUT>) {$mantext .= $_;}
    $status = $?;
  } else {
    close(STDIN);
    open(STDERR, '>&STDOUT');
    if ($section) {exec('/usr/bin/man', $section, $page);}
    else {exec('/usr/bin/man', $page);}
  }
# now reformat $mantext and display it.
[download]

Now, there are some nice security plusses in this code - the use of the many-arg form of exec, for example, avoids a whole host of shell-escaping issues. However, this gives a potential attacker shell access on any system whose man command allows the -P option. (quid vide) All an attacker needs to do is pass in

  section=-P/usr/bin/whatever%20command%20I%20want&page=cat
[download]

as part of the url, and their command will be executed. (And fed the "cat" manpage as input, but that's immaterial)

The general lesson here is that options change the behavior of external commands in ways you don't expect; don't allow the user to send options to external commands. Fortunately, with almost every unix command passing a '--' will prevent subsequent arguments from being interpreted as options, so a fixed version of the above code could read:

# $page and $section are parameters from the user that have been clean
+ed of 0 bytes and obvious control characters
  my $mantext = ''; my $status;
  my $pid = open(KID_STDOUT, "-|");
  if (not defined $pid) {
    die "cannot fork: $!; bailing out";
  }
  if ($pid) { ## parent
    while(<KID_STDOUT>) {$mantext .= $_;}
    $status = $?;
  } else {
    close(STDIN);
    open(STDERR, '>&STDOUT');
    if ($section) {exec('/usr/bin/man', '--', $section, $page);}
    else {exec('/usr/bin/man', '--', $page);}
  }
# now reformat $mantext and display it.
[download]

As an aside, note that the following code contains the same hole as the initial code:

  my $qpage = quotemeta($page);
  my $qsect = quotemeta($section || '');
  exec("/usr/bin/man $qsect $qpage");
[download]

The issue is not shell escaping - the issue is that when calling external commands, be aware that many commands use arguments beginning with "-" to mean "radically alter your behavior in some fashion". This leads to behavior you can't predict ahead of time, which means that guarding against it is almost impossible if you allow options to be passed along.

Note that on an MS windows platform, (and, I suppose, on VMS too) some external commands may treat arguments beginning with '/' as options. Unfortunately, I don't know of any standard way to prevent that as with the '--' common on unix; on those platforms you'll just have to be careful to strip leading / characters in cases where the variables are being used in a way that could pass unwanted options to an external command.

--
@/=map{[/./g]}qw/.h_nJ Xapou cets krht ele_ r_ra/;
map{y/X_/\n /;print}map{pop@$_}@/for@/
[download]

In reply to Re: Security techniques every programmer should know by fizbin
in thread Security techniques every programmer should know by Juerd

Are you posting in the right place? Check out Where do I post X? to know for sure.
Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
Want more info? How to link or How to display code and escape characters are good places to start.


Welcome to the Monastery
	PerlMonks