Beefy Boxes and Bandwidth Generously Provided by pair Networks Frank
Pathologically Eclectic Rubbish Lister
 
PerlMonks  

Escaping %params

by DaisyLou (Acolyte)
on Jan 20, 2014 at 22:07 UTC ( #1071383=perlquestion: print w/ replies, xml ) Need Help??
DaisyLou has asked for the wisdom of the Perl Monks concerning the following question:

OK OK. You guys win. You're right. I will not bother with the regex stuff and will used prepared statements. Thanks to all for your insistence. Better to do it right the first (well, second) time than to hack in something that'll have to be replaced and will probably cause more issues.
my $cgi = CGI->new(); my %params = $cgi->Vars(); #want to escape %params here my $escapedId = $params{id};
The keys and values in params will vary (ie, cannot always assume that "id" will be the name -- looking for a routine that's generic here). I order to prevent SQL injection, I need to escape any values in the %params that contain characters like '. Is there a simple way to go through params, and change all of the values inside of params? Edit: I know this is not perfect and that I should be using parameters -- just looking for something until a more permanent soultion is in place.

Comment on Escaping %params
Download Code
Re: Escaping %params
by chromatic (Archbishop) on Jan 20, 2014 at 22:13 UTC

    Rather than escape values yourself, using SQL placeholders lets your database driver escape the values for you. It's a lot easier and safer. If you were to post some example database code, someone would surely show you how to use placeholders instead.


    Improve your skills with Modern Perl: the free book.

      You are right, and in the long-run, that's what will probably be done, but that's a lot of work and testing. This is to "get us by" in the meantime.

        Not to sound like a grumpy old man, but "gets us by" is exactly how insecure systems are created and forgotten. If your goal as stated is to 'prevent SQL injection' then there is only one answer: use place holders.

Re: Escaping %params
by hippo (Chaplain) on Jan 20, 2014 at 22:18 UTC
      I know it's not perfect -- just looking for something untikl we get a more permanent soultion in place like what you suggested.
Re: Escaping %params
by choroba (Abbot) on Jan 20, 2014 at 23:33 UTC
    You can start playing with something like the following:
    #!/usr/bin/perl use warnings; use strict; use Data::Dumper; sub safer { my $hash = shift; my %safer; while (my ($k, $v) = each %$hash) { s/'/\'/g for $k, $v; $safer{$k} = $v; } return %safer } my %params = ('a b' => 'c d', "a'b" => "c'd", 'a"b' => 'c"d', 'a`b' => 'c`d', "a\\'b" => "a\\'b", ); my %safer = safer(\%params); print Dumper \%safer;
    لսႽ ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ
      Thank you! Is it necessary to have
      my %safer = safer(\%params);
      or would
      safe(\%params); print Dumper \%params;
      be equally good?

        Have you tried?

        It might be possible if you changed the definition of the function. You can always do
        %params = safer(\%params);
        لսႽ ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ
Re: Escaping %params
by tangent (Chaplain) on Jan 21, 2014 at 00:11 UTC
    As mentioned in the CB $cgi->Vars may cause problems - a safer way to build the params hash is:
    my %params = map { $_ => $cgi->param($_) } $cgi->param;
    And beware of params that have more than one value, e.g. checkbox groups

    Update: to allow for params with multiple values:

    my %params; for my $name ($cgi->param) { my @values = $cgi->param($name); $params{$name} = @values > 1 ? \@values : $values[0]; }
    When further processing %params you will need to check if $param{$name} is an array reference.
      Thank you... Interestingly, when I use this code:
      use warnings; use strict; use CGI; use Data::Dumper; print "Content-type: text/html\n\n"; my $cgi = CGI->new(); my %params = map { $_ => $cgi->param($_) || '' } $cgi->param; print Dumper \%params;
      The output is when I pass test.pl?a=\'b is:
      $VAR1 = { 'a' => '\\\'b' };
      ... so it looks like something in the stack is already taking care of the escaping for me. Am I worrying about nothing?
        Data::Dumper is doing that - try this:
        for my $param (keys %params) { print "$param: $params{$param}<br>" }

        Data::Dumper does perl-escaping (defaults have some caveats)

        Data::Dump::pp() does better perl-escaping by default

        Neither ddumper does HTML-escaping

        You can alway do  my $cgi = CGI->new; print $cgi->header, $cgi->Dump ; to see whats inside $query

        s/_/\_/g

        s/_/\_/g does nothing. You probably want  s/_/\\_/g

        >perl -wMstrict -le "$_ = 'a_b__c___'; print qq{before: '$_'}; ;; s/_/\_/g; print qq{after: '$_'}; " before: 'a_b__c___' after: 'a_b__c___'

        But please allow me to add my voice to the chorus imploring you to Just Use Placeholders!.

      I've distilled all this advice this as best I can.
      ============ something.lib ============ sub safer { my $hash = shift; my %safer; while (my ($k, $v) = each %$hash) { s/\\//g for $k, $v; s/0x00//g for $k, $v; s/0x08//g for $k, $v; s/0x09//g for $k, $v; s/0x0a/\n/g for $k, $v; s/0x0d/\r/g for $k, $v; s/"/\\"/g for $k, $v; s/%/\\%/g for $k, $v; s/'/\\'/g for $k, $v; s/_/\_/g for $k, $v; $safer{$k} = $v; } return %safer; } ================ something.cgi... ================ use warnings; use strict; use CGI; use CGI::Carp; print "Content-type: text/html\n\n"; # marker my $cgi = CGI->new(); $cgi->param; my %params; for my $name ($cgi->param) { my @values = $cgi->param($name); $params{$name} = @values > 1 ? \@values : $values[0]; } %params=safer(\%params); # marker for my $param (keys %params) { print "$param: $params{$param}<br>" }
      The stuff between the two markers will replace the existing
      sub main { my $cgi = CGI->new(); my %params = $cgi->Vars();
      ... in the existing scripts. This is running under mod-perl (w/ regcooker). Are there any "gotchas" I should be aware of here? Thanks to all you monks for all your help!

        A problem is with CGI->Vars , you never want to use CGI->Vars, CGI->Vars is for perl4, Vars mangles (encodes, serializes, packs, implodes) the data, its backwards compatibility for some 1993 stuff

        You want "escapeHTML" from CGI.pm

Re: Escaping %params
by Jenda (Abbot) on Jan 21, 2014 at 15:26 UTC

    This exactly the wrong thing to do. You gain false sense of security and end up with invalid data in the database and displayed. We all remember PHP-based sites displaying thing's like d\'Artagnan ...

    Jenda
    Enoch was right!
    Enjoy the last years of Rome.

Re: Escaping %params
by trwww (Curate) on Jan 21, 2014 at 16:27 UTC
Re: Escaping %params
by FloydATC (Hermit) on Jan 21, 2014 at 16:44 UTC

    Every minute spent quoting variables by hand and working around the endless stream of problems it will inevitably lead to, is a complete waste.

    Just rewrite the queries to use placeholders, because sooner or later that's what you will end up having to do. Not only does it solve all problems related to SQL injection and special characters, it also makes your code easier to read and maintain. With most databases it will even improve performance.

    And no, it's not difficult either. Just replace this

    my $sth = $dbh->prepare(" SELECT * FROM foo WHERE bar=$qstring1 AND baz=$qstring2 "); $sth->execute();

    with this

    my $sth = $dbh->prepare(" SELECT * FROM foo WHERE bar=? AND baz=? "); $sth->execute($string1, $string2);

    ...it's really that simple!

    Now, if you really REALLY must quote each variable because you have to build the queries on the fly, don't do the quoting yourself using regular expressions. Instead, use the DBD "quote" method conveniently provided for you. The following code works for simple cases:

    my %quoted = map { $_ => $dbh->quote($params{$_}) } keys %params;

    It produces a copy of the original hash, with all values properly quoted for safe use by the database you happen to be working with.

    -- FloydATC

    Time flies when you don't know what you're doing

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://1071383]
Approved by atcroft
Front-paged by ww
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others perusing the Monastery: (26)
As of 2014-04-17 15:54 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    April first is:







    Results (453 votes), past polls