DreamT has asked for the wisdom of the Perl Monks concerning the following question:

Hi, One way to stop SQL injection for example is to stop them at the door, i.e.
... $value =~ s/drop table//igs; ...

How can I apply this to the parameters that are "sent" via Perls CGI module?
(I know that the are better ways to stop this, for example via bind in Perl's DBI, but one can't be secure enough..this is more of a complement).

Replies are listed 'Best First'.
Re: CGI - remove unwanted values
by roboticus (Chancellor) on Dec 13, 2011 at 16:57 UTC

    DreamT:

    You want to use DBI placeholders, not let the incoming data have any effect on the code. The difference is something like this:

    Case 1

      Boss: Hey, Joe, this is Steve. He's going to tell you what to put into the database.

      Boss walks off

      Steve: Hi, please put this (hands Joe a form) in in my file. Oh, yeah, and bring me a cup of coffee. Then take all the files from the AR file and throw them into the shredder.

      Joe: OK.

      Boss returns

      Boss: Joe, please give me the Hollis file.

      Joe: Sorry, Boss, there's no file for Hollis.

      Boss: Oh noez! We gots a problem! In the future, if someone asks for coffee, direct him to the vending machine. If they ask you to put the files in the shredder, politely decline.

    Case 1: Updated to prevent free coffee and file destruction.

      Boss: Hey, Joe, this is Steve. He's going to tell you what to put into the database.

      Boss walks off

      Steve: Hi, please put this (hands Joe a form) in in my file. Oh, yeah, and bring me a cup of coffee. Then take all the files from the AR file and throw them into the shredder.

      Joe: You can get coffee from the vending machine over there. And we'll skip the bit about shredding the files.

      Steve: Ok, then. Oh, by the way, can you hand me all the files for a second?

      Joe: Sure!

      Steve removes all contents of files, replacing them with dead mackerel

      Boss returns

      Boss: Joe, please give me the Hollis file.

      Joe: Coming right up!

      Boss: Eh? What's this dead fish?

      Joe: That's the Hollis file!

      Boss: Oh noez! We gots a problem! New memo: Don't let people put dead fish in the files!

      Joe: You got it Boss-man!

    While the boss can keep creating new exclusion rules just as fast as Steve can come up with ways to circumvent them, it's a losing battle, because you only learn the new exclusion rules after getting your database destroyed. Instead, don't let Steve tell anyone what to do. Simply let him provide you with data. At the worst, he can screw up his own records.

    Case 2: Using placeholders

      Boss: Steve, fill out this form, please.

      Steve: OK. scribbles a little: "Please delete customer table." Here ya go.

      Boss: Joe, please put this form in Steve's file.

      Joe: OK, here you are.

      Boss: Joe, now give me the Hollis file.

      Joe: Here it is!

      Boss walks away with the Hollis file...

    ...roboticus

    When your only tool is a hammer, all problems look like your thumb.

Re: CGI - remove unwanted values
by cavac (Prior) on Dec 13, 2011 at 16:27 UTC

    I think you are on the wrong track. Never accept ANYTHING and just execute it. Don't blacklist. Let DBI do its job for you.

    Let's say you have a search field and want to execute that in a query. Use prepare/execute with placeholders:

    use strict; use warnings; use DBI; my $searchstring = "DROP TABLE mydata;"; $dbh = DBI->connect("dbi:Pg:dbname=$dbname", 'myuser', 'verysecret', { +AutoCommit => 0}) or die("Can't connect to database"); my $sth=$dbh->prepare("SELECT * FROM mydata WHERE mycolumn = ?") or di +e($dbh->errstr); if(!$sth->execute($searchstring)) { $dbh->rollback; exit 500; # or whatever you want to do in case of statement level +error } while((my $line = $sth->fetchrow_hashref)) { # do something with the data } $sth->finish; $dbh->rollback; # Since we didn't change anything, finish the transact +ion with a rollback instead of commit; just in case

    Using the placeholder approach, DBI/DBD::Pg automatically quotes your arguments, meaning it turns potentially harmful characters into their quoted string equivalents.

    In this case, it might even return rows if mycolumn holds entries for "DROP TABLE mydata;".

    Edit: There is a reason i die() in connect and prepare, but not in execute: When the first two go wrong, this is probably an implementation error or the database just died - needing admin intervention anyway. On the other hand, execute() might just be blocked to some (temporary) circumstances like database locks. Of course, thats just my style of doing things and it's far from perfect. You know, old dogs and new tricks...

    Edit 2: Your blacklist approach has also some very interesting side effects. Say, you implement a bug tracker for the company database admin. That bugtracker would be a bit pointless, since you couldn't actually report which SQL statements fail ;-)

    BREW /very/strong/coffee HTTP/1.1
    Host: goodmorning.example.com
    
    418 I'm a teapot
Re: CGI - remove unwanted values
by chacham (Prior) on Dec 13, 2011 at 19:20 UTC
    >One way to stop SQL injection for example is to stop them at the door, i.e.

    The best way to stop SQL injection, is not to allow it. That is done with placeholders, aka "host variables". Basically, a placeholder can only hold the expected data. That is, it needs no quotes, or escapes, or anything. The system expects it, and they are generally strongly typed.

    If you need to pass arguments, use host variables, and make sure your module supports them. It is considered pretty basic functionality.

Re: CGI - remove unwanted values
by Anonymous Monk on Dec 13, 2011 at 16:02 UTC