in reply to CGI and Traceroute

Lots of good advice, but I'm surprised no one mentioned avoiding the shell entirely by using the multi-argument forms of system() or exec() to call traceroute directly.

Put together with the above advice and some sugar, you get something like ...

#!/usr/bin/perl -wT

$|++;

use strict;
use CGI qw( header start_html end_html param p );

print header, start_html( "RavenGate TraceRoute Results" );

if ( param('trace') and param('trace') =~ /^[-.0-9a-zA-Z]+$/ ) {
    print p( "Hello There- I am writing this from "
           . "scratch so please be patient. Thanks!" ),
        "<pre>";

    system( "/usr/sbin/traceroute", param('trace') );

    print "</pre>";
    print p( "Oops, there was a problem running traceroute" ) if $?;
} elsif ( param('trace') ) {
    print p( "Oops,", param('trace'), "contains illegal characters" );
} else {
    print p( "Oops, you forgot to give me a host to trace to." );
}

[download]

As for that one line that "doesn't work", you need to escape the pipe (ie, $value =~ s /\|/ /g;).

Replies are listed 'Best First'.
Possible Security Hole (was RE: Re: CGI and Traceroute)
by merlyn (Sage) on Oct 19, 2000 at 08:11 UTC 
    ...
if ( param('trace') and param('trace') =~ /^[-.0-9a-zA-Z]+$/ ) { 
    print p( "Hello There- I am writing this from " 
           . "scratch so please be patient. Thanks!" ), 
        "<pre>"; 
 
    system( "/usr/sbin/traceroute", param('trace') ); 
...

    [download]
    Hoo boy. Watch out. You've used param() in both a scalar and a list context. In your security test, you look for things of interest, but in a scalar context, which means you are testing the first param value for trace if multiple values are defined. But then, you suck down all the param-values for trace when you fire off the traceroute. Guess what? You are now passing untested data to traceroute!

    And given that there were some buffer overflow problems with traceroute if I recall my BUGTRAQ listings, giving root access to local users, you've just handed the keys to the kingdom to all-comers. Whee!

    Security - it's not just for breakfast any more.

    -- Randal L. Schwartz, Perl hacker

[reply]
[d/l]
      Would this be a better approach?

      Update: corrected the code (thanks merlyn); sorry for the delay...

         # assuming -T
   ...
   my $param_cmd = param( 'trace' );
   if ( $param_cmd ) and ( $param_cmd =~ /^[-.0-9a-zA-Z]+$/ )
   {
      $param_cmd = $1;
      print p( "Hello There- I am writing this from " 
               . "scratch so please be patient. Thanks!" ), 
               "<pre>"; 
        
      system( "/usr/sbin/traceroute", $param_cmd ); 
      ...
   }

      [download]

      (Sorry it this seems basic, but I'm trying to make sure I've learned the right things from the various FAQ's and nodes on the subject...)

[reply]
[d/l]
        This may work for you:

        use Untaint;

my @dirty_params = param('trace'); # your array
my @clean_params = untaint(qr(^[-.0-9a-zA-Z]+$), \@dirty_params);

        [download]

        Assuming all members of your array are launderable, you will be returned an array of clean values.

        Cheers,
        KM

[reply]
[d/l]

        Your parens are a bit off in the if. Try ...

        if ( $param_cmd and $param_cmd =~ /^([-.0-9a-zA-Z]+)$/ ) {

        Note the addition of parens in the regexp so that you save it in $1, as the way you had it would have set $param_cmd to be undefined.

        Also, there's no need for qw() which would've passed a different argument than you expected ...

        system( "/usr/sbin/traceroute", $param_cmd );

          --k.

        ( Ain't life^H^H^H^Hsecurity a biiyatch? :)
[reply]
[d/l]
[select]
[reply]
[d/l]

In Section Seekers of Perl Wisdom