[Omaha.pm] SQL Attack exception

["Kenneth Thompson" <kthompson@omnihotels.com>]

One problem with method 2 is that it's an all or nothing proposition. If
you potentially have mixed fields (something that inputs to some SQL AND
a file upload), you have to break your process into 2 steps.

Perhaps a compromise would be to pass in the fields you don't want
checked/cleaned on instantiation?


my $page = new View::Web::Page(Globals=>$Globals,SafeList=>['File1',
'File2']);

in the constructor:

  my ($self, %args) = @_;
  my @SafeList = @$args{'SafeList'};
  my (%Ignored, $param); 
  foreach $param (@SafeList) {$Ignored{$param} = 1};   
  
  foreach my $param ($q->param()) {
     # Strip out all wacky characters to prevent SQL injections
     #
     next ($IgnoreParms{$param}); #ignored - bail now
     my $value = $q->param($param); #Not ignored.. clean me up Scotty
     $value =~ s/[`;'"\\]//g;
     $q->delete($param);	  
     ...etc...



-----Original Message-----
Message: 1
Date: Thu, 28 Jul 2005 14:18:12 -0500
From: "Jay Hannah" <jhannah@omnihotels.com>
Subject: Re: [Omaha.pm] Thoughts?
To: <omaha-pm@pm.org>
Message-ID: <200507281916.j6SJGlic015933@omares-email.omnihotels.com>
Content-Type: text/plain;	charset="us-ascii"


I like idea #2, activated when you *know* you're not going to do
anything exploitable.

But I don't like the switch RawCGI=>1. 

I'd vote for the RARE use of:

   my $page = new View::Web::Page(Globals=>$Globals,Safe=>0);

In the constructor default Safe to 1 (on/true).

   $Safe = 1 unless (defined $Safe);
    foreach my $param ($q->param()) {
   if ($Safe) {        # Strip out all wacky characters to prevent SQL
injections
   ...etc...

$0.02,

j



> So, I ran into an issue using View::Web::Page and file uploads. Jay 
> helped point me to a function of the class that "cleans" all the 
> q->params() to stop sql attacks.
> Unfortunately, it also strips all the backslashes out of my filepath 
> that IE pukes into the form-data (mozilla conveniently removes all but

> the filename in formposts) making it difficult to parse the filename.
> 
>  
> 
> I figure there's 2 ways to address this without reducing the 
> attack consideration:
> 
>  
> 
> 1. Specifically ignore 'special' params :
>     foreach my $param ($q->param()) {
> 
>       # Strip out all wacky characters to prevent SQL injections
>       #
>       If ($param ne 'fileupload') {
>         my $value = $q->param($param);
>         $value =~ s/[`;'"\\]//g;
>         $q->delete($param);
>         $q->param($param,$value);
>         if ($param =~ /^(view|edit|update|delete|insert)__/) {
>           my @arr = split /__/, $param;
>           $pagemode = shift @arr;
>           $pagename = shift @arr;
>           $pageid   = join('__', @arr);
>           last;
>         }
>       }
>     }
> 
>  
> 
> 2. instantiating it like this
> 
> my $page    = new View::Web::Page(Globals=>$Globals,RawCGI=>1);
> 
> and adding an if around this block of code 
> 
>   if (!$RawCGI) {
>     foreach my $param ($q->param()) {
>       # Strip out all wacky characters to prevent SQL injections
>       #
>       my $value = $q->param($param);
>       $value =~ s/[`;'"\\]//g;
>       $q->delete($param);
>       $q->param($param,$value);
>       if ($param =~ /^(view|edit|update|delete|insert)__/) {
>         my @arr = split /__/, $param;
>         $pagemode = shift @arr;
>         $pagename = shift @arr;
>         $pageid   = join('__', @arr);
>         last;
>       }
>     }
>   }
>  
> 
> Thoughts?