Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl Monk, Perl Meditation
 
PerlMonks  

Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite

by talexb (Canon)
on Jan 09, 2008 at 17:29 UTC ( #661423=perlmeditation: print w/ replies, xml ) Need Help??

This is a followup to my original question Preventing SQL injection attacks: are -T and placeholders not enough?. When I run the following code on my system..

#!/usr/bin/perl -w use DBI; # Test SQL injection attack. The test table 'jobs' was created with # the command 'create table jobs (j_id integer, j_value text);' in # all three databases. my @databases = ( 'mysql:test', 'Pg:dbname=test', 'SQLite:test.sq3' ); my @data = ( { id => 44, value => "Some benign text" }, { id => 55, value => "Just regular data" }, { id => 66, value => "Evil data');DELETE FROM jobs;" } ); { foreach my $thisDbName ( @databases ) { my $dbh = DBI->connect("DBI:$thisDbName", undef, undef ) or die "Unable to connect to $thisDbName: " . $DBI::errstr; print "Connected OK to $thisDbName.\n"; # Clean up test table before we start .. my $cmd = "DELETE FROM jobs"; my $sth = $dbh->prepare($cmd); print "Clear out existing data from the test table ..\n"; $sth->execute or die "Problem executing $cmd: " . $sth->errstr; $cmd = "INSERT INTO jobs (j_id, j_value) VALUES (?,?)"; $sth = $dbh->prepare($cmd); # Add test data into table .. foreach my $hashref ( @data ) { print "Add " ."($hashref->{'id'},$hashref->{'value'})" ." to the test table ..\n"; $sth->execute($hashref->{'id'}, $hashref->{'value'}) or die "Problem executing $cmd: " . $sth->errstr; } # Dump out the resulting tables. $cmd = "SELECT * FROM jobs"; $sth = $dbh->prepare($cmd); print "Dump out the result.\n"; $sth->execute or die "Problem executing $cmd: " . $sth->errstr; DBI::dump_results($sth); print "\n"; } }

I get the following results:

Connected OK to mysql:test. Clear out existing data from the test table .. Add (44,Some benign text) to the test table .. Add (55,Just regular data) to the test table .. Add (66,Evil data');DELETE FROM jobs;) to the test table .. Dump out the result. '44', 'Some benign text' '55', 'Just regular data' '66', 'Evil data');DELETE FROM jobs;' 3 rows Connected OK to Pg:dbname=test. Clear out existing data from the test table .. Add (44,Some benign text) to the test table .. Add (55,Just regular data) to the test table .. Add (66,Evil data');DELETE FROM jobs;) to the test table .. Dump out the result. '44', 'Some benign text' '55', 'Just regular data' '66', 'Evil data');DELETE FROM jobs;' 3 rows Connected OK to SQLite:test.sq3. Clear out existing data from the test table .. Add (44,Some benign text) to the test table .. Add (55,Just regular data) to the test table .. Add (66,Evil data');DELETE FROM jobs;) to the test table .. Dump out the result. 44, 'Some benign text' 55, 'Just regular data' 66, 'Evil data');DELETE FROM jobs;' 3 rows

So, as far as I'm concerned, placeholders are enough to prevent SQL injection attacks for MySQL, PostgreSQL and SQLite. If anyone can point out something obvious that I've missed, please let me know.

Update: Thanks to the monks who have contributed, we now have the following results:
PassFail
  • MySQL, PostgreSQL and SQLite (from this node)
  • Oracle 10g olus
  • MS Access and SQLServer (both 2000, I believe) rhesa
  • IBM UDB version 9.5.0 on Linux 2.6.9 i386 andreas1234567
  • CSV, DBM -- In theory that means that should pass with any of the SQL::Statement DBDs. jZed
  • DBD::Sybase 1.08, Sybase ASE 15.0.2 and Sybase OpenClient 15 mpeppler
  • Nothing yet.

Update 2: As per tye's reply below, please note that this is obviously not a rigorous test that proves conclusively that placeholders always prevent SQL injection attacks, but rather a quick check that an obvious test does indeed work properly.

Alex / talexb / Toronto

"Groklaw is the open-source mentality applied to legal research" ~ Linus Torvalds

Comment on Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
Select or Download Code
Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by olus (Curate) on Jan 09, 2008 at 18:48 UTC
    Ran your code for an Oracle 10g and got the same results.
    my $dbh = DBI->connect('dbi:Oracle:', 'usr/pass@service_name', '') or + die DBI->errstr; .... Connected OK to Oracle. Clear out existing data from the test table .. Add (44,Some benign text) to the test table .. Add (55,Just regular data) to the test table .. Add (66,Evil data');DELETE FROM jobs;) to the test table .. Dump out the result. '44', 'Some benign text' '55', 'Just regular data' '66', 'Evil data');DELETE FROM jobs;' 3 rows

      Excellent!!! Thanks for doing that. Anyone else?

      Alex / talexb / Toronto

      "Groklaw is the open-source mentality applied to legal research" ~ Linus Torvalds

Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by moritz (Cardinal) on Jan 09, 2008 at 19:07 UTC
    Nice work (although the outcome is not unexpected ;).

    There are other potential security risks, though. For example if you use an ORM mapper (like DBIx::Class or Rose::DB) and construct a complicated query, you have to know exactly which arguments are parsed as SQL and which aren't.

    But if you really stick to plain DBI with placeholders you don't have to worry very much about SQL injection.

    You still have to consider possible DoS attacks, but that's usually not as bad as SQL injection.

        Nice work (although the outcome is not unexpected ;).

      Thanks -- I was hoping to see the result that I did actually see, but I was very worried. I've always assumed that placeholders are safe. With the uncertainty, I did what any engineer or scientist would do -- I conducted an experiment to find out what was really happing.

      The modules like DBIxC and Rose sound nice, and I will try DBIxC again -- ten years ago I was putting raw HTML into my Perl CGIs, and that's a no-no for me now. Perhaps in another ten years there won't be any SQL in my modules because I'm using DBIxC. Or maybe I'll reach that state of Nirvana sooner. I'll see.

        But if you really stick to plain DBI with placeholders you don't have to worry very much about SQL injection.

      Yeah. :)

      Alex / talexb / Toronto

      "Groklaw is the open-source mentality applied to legal research" ~ Linus Torvalds

        but I was very worried

        About a single quote character causing a problem? Wow, you must think that the DBD modules are almost completely untested?

        Though I don't see how you jump from preventing one trivial SQL injection attack to the grand conclusion that all injection attacks will surely be prevented. But placeholders aren't exactly rocket surgery so I'd be rather surprised if somebody could find character strings that are not properly handled by a DBD (ignoring character encoding problems which are still quite a nuisance), much less allowing SQL to be injected.

        But I've certainly found plenty of problems with DBD placeholders. None of them were the type that would allow for SQL injection. Most of them were in various versions of DBD::ODBC and include such things as not being able to handle dates without jumping through extra hoops of complexity (or just not being able to use dates via placeholders at all) and more vague problems of queries just not matching properly until I replaced the placeholders with simple string templates using DBD's ->quote(). There were also some problems with DBD::mysql and quotes around numerical values in some situations.

        Update: Also consider that your audience's FUD may be related to PHP which, from what I've heard, does some rather unreliable "magic" trying to automatically deal with single quotes in data to/from databases without making a clear distinction between the quoted strings and the (unquoted) string values and that this "magic" doesn't work very well.

        - tye        

      There are other potential security risks, though. For example if you use an ORM mapper (like DBIx::Class or Rose::DB) and construct a complicated query, you have to know exactly which arguments are parsed as SQL and which aren't.

      I am not sure I understand this comment. As far as I know DBIx::Class uses placeholders internally so you don't have to worry about things like this. As for the "you have to know exactly which arguments are parsed as SQL" part of the comment, it treats *all* of its arguments as perl values and not SQL values unless you use the options to pass in a raw SQL string, in which case its your responsibility to make sure it doesnt open you up to risks. I can't speak for Rose::DB because I don't use it, but I would be shocked if the author didn't take all this stuff into account too.

      But if you really stick to plain DBI with placeholders you don't have to worry very much about SQL injection.

      Yes, you also have to manage your own SQL -> Object conversion, and write a LOT more code, and more code == more bugs. Modules like DBIx::Class and Rose::DB have been pretty thoroughly tested by lots of users in serious production applications (I can count 4 we have here at $work), if they didn't handle simple security stuff like this correctly then they would have been dismissed as crap long ago.

      -stvn
        As for the "you have to know exactly which arguments are parsed as SQL" part of the comment, it treats *all* of its arguments as perl values and not SQL values unless you use the options to pass in a raw SQL string, in which case its your responsibility to make sure it doesnt open you up to risks. I can't speak for Rose::DB because I don't use it, but I would be shocked if the author didn't take all this stuff into account too.

        Your instincts were right :)

        Rose::DB::Object (note: not Rose::DB, which is a db abstraction layer used by the ORM, Rose::DB::Object) uses bind values everywhere, except in cases where the DBD::* driver requires that values be "inlined." There is no ambiguity in these cases as the values that are allowed to be are specific for each column type/database combination.

        For example, Informix supports a syntax like this:

        SELECT * FROM t1 WHERE somedate > CURRENT;

        where "somedate" is a DATE column type. Unfortunately, DBD::Informix chokes on this:

        $sth = $dbh->prepare('SELECT * FROM t1 WHERE somedate = ?'); $sth->execute('CURRENT'); # Boom: error!

        because it considers the above the equivalent of comparing somedate to the string "CURRENT", as if it were:

        SELECT * FROM t1 WHERE somedate > 'CURRENT';

        Then the database itself throws an error since it refuses to compare a DATE column to a string. (Other databases are more lenient about this kind of thing.)

        Anyway, the upshot is that, if you want to use CURRENT (or any other "special" server-evaluated value), you must inline it directly into the query passed to DBI.

        In Rose::DB::Object, such values are called "keywords" and are automatically inlined. So if you add this clause to a Rose::DB::Object::Manager query:

        somedate => { gt => 'CURRENT' },

        it'll be smart enough to realize that a) "somedate" is a DATE column and b) "CURRENT" is a valid keyword for DATE columns in Informix, so it'll inline the value instead of using a bind parameter.

        Again, these lists of keywords are specific to each column-type/database combination, so the word "CURRENT" would not be inlined if you were connected to a MySQL database, for example.

        Also note the lack of ambiguity: it's clear that there's only one reasonable meaning of CURRENT when used as a comparison value for a DATE column. IOW, there's never a reason for it not to be inlined in this case, so the auto-inlining is never a hindrance.

        Finally, you can always explicitly force any value to be taken as a literal and inlined by passing it as a scalar reference:

        somecol => { gt => \q(iknowwhatimdoing) },

        That'll produce SQL like this, with no "?" placeholders, quoting, or bind values:

        somecol > iknowwhatimdoing

        Obviously, you'd only do this in very specific cases when you're sure what you're doing is safe :)

        If you treat your database as a dumb object store I bet my supadances against your socks that you are using the database very very inefficiently, bogging it down with unoptimized (and unoptimizable) ad-hoc queries, fetching many times more data that you actually need etc.

        All I want from my database access layer is to let me call the stored procedures (few of them one statement only) without much fuss, thank you very much.

Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by rhesa (Vicar) on Jan 09, 2008 at 19:33 UTC
    Since I happened to be working on Windows for a client, I thought I'd check there.

    Verified for MS Access and SQLServer (both 2000, I believe)

    Driver versions: DBD::ODBC: 1.07 DBD::ADO: 2.84 Connected OK to ODBC:sqlservertest. Clear out existing data from the test table .. Add (44,Some benign text) to the test table .. Add (55,Just regular data) to the test table .. Add (66,Evil data');DELETE FROM jobs;) to the test table .. Dump out the result. '44', 'Some benign text' '55', 'Just regular data' '66', 'Evil data');DELETE FROM jobs;' 3 rows Connected OK to ADO:Provider=Microsoft.Jet.OLEDB.4.0;Data Source=d:/te +mp/msaccess2000test.mdb. Clear out existing data from the test table .. Add (44,Some benign text) to the test table .. Add (55,Just regular data) to the test table .. Add (66,Evil data');DELETE FROM jobs;) to the test table .. Dump out the result. 44, 'Some benign text' 55, 'Just regular data' 66, 'Evil data');DELETE FROM jobs;' 3 rows
Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by andreas1234567 (Vicar) on Jan 09, 2008 at 19:33 UTC
    Ran your code on IBM UDB version 9.5.0 on Linux 2.6.9 i386.
    $ perl -w 661423.pl Connected OK to DBD::DB2::VIPER. Clear out existing data from the test table .. Add (44,Some benign text) to the test table .. Add (55,Just regular data) to the test table .. Add (66,Evil data');DELETE FROM jobs;) to the test table .. Dump out the result. '44', 'Some benign text' '55', 'Just regular data' '66', 'Evil data');DELETE FROM jobs;' 3 rows
    However, I would in most cases favor stored procedures as main way of interacting with a database. Benefits includes both security (which includes preventing SQL injection attacks - simply because you don't get to create dynamic SQL) and performance.
    --
    Andreas
Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by sundialsvc4 (Monsignor) on Jan 10, 2008 at 03:06 UTC

    Here are my thoughts...

    1. You should not rely upon what a particular DBI-implementation actually does with “a parameterized query.”
    2. Nevertheless... you should know your own business. You should know what parameters you are expecting, and for each one you should know (a) that the value is “a scalar” and (b) what regular-expression pattern it should match.

    Both of these considerations will be “specific to your application,” and therefore you should bear the first level of responsibility for ensuring conformance to them.

Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by aquarium (Curate) on Jan 10, 2008 at 03:18 UTC
    if you want to be fairly sure that your code is SQL injection safe against typical attacks, then you should use typical attacks to test with. Most hackers use an up-to-date bundle of tricks, typically already in a script, to try to cause harm...and don't bother hand hacking. As such, the trivial test cases presented do not represent a typical SQL injection hackers bundle, by any stretch of imagination.
    also, to help prevent SQL injection...normally you also untaint the data by an inclusion regex. e.g.
    bad_input() if($cityname !~ /^[a-zA-Z .,]+$/);
    ..and never untaint by disallowing banned characters instead. you never know if your banned character list is complete.
    the hardest line to type correctly is: stty erase ^H
      You said that Most hackers use an up-to-date bundle of tricks, typically already in a script, to try to cause harm...and don't bother hand hacking. I didn't have much success looking that up in Google. Do you have any suggestion on resources/modules providing test cases of SQL injection or any other type of security threat? Such a module would be great for testing code safety or queries safety.

      BTW, when/if possible, it always seems safer to me to check inputs in a "white list" fashion. If you check that inputs contain only letters, numbers and underscores and don't exceed a certain length, that would probably increase security by a great deal.
Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite (and DBD::CSV and DBD::DBM)
by jZed (Prior) on Jan 10, 2008 at 06:53 UTC
    Tests passed with this:
    @databases = qw ( CSV: DBM: );
    In theory that means that should pass with any of the SQL::Statement DBDs.
Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by mpeppler (Vicar) on Jan 10, 2008 at 07:53 UTC
    Just to be complete - works fine with DBD::Sybase 1.08, Sybase ASE 15.0.2 and Sybase OpenClient 15.

    Michael

Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by sundialsvc4 (Monsignor) on Jan 10, 2008 at 18:23 UTC

    As a general rule, you need to use regular-expressions to verify that all of your parameters conform to the expected format, and you need to be sure that they are, in fact, scalars. (Multiple occurrences of the same token in a GET-string can create an array-type value in some cases.)

    Note that your patterns should describe exactly what you will accept. Don't try to write patterns to filter-out or to recognize what you wish to reject. “Think positive.” The pattern should consider not only character-types but also plausible length-ranges. If the patterns occur consistently throughout the application, put all of them into their own library unit that you can “use.”

    You also need to be sure that the values come from the correct source... GET or POST.

    Finally, consider using verification strings on, say, your hotlinks. This is a GET-parameter that you've added to the URL, consisting of (say...) an SHA1 hash of the URL-value, perhaps the session-id, and an unknown-to-the-attacker random string. If your program gets a URL-reference that does not contain a valid verification string, the request is rejected. (Naturally, there's plenty of CPAN material available to do this.)

    This approach will work regardless of what kind of back-end database (or other data store) that you intend to use. If these tests are put in a central location at the dispatching heart of the application, they will apply consistently throughout the code and thus protect all of it.

Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by ikegami (Pope) on Oct 11, 2008 at 01:25 UTC
    What about testing with the string
    my $evil = join '', map chr, 0..255;

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others making s'mores by the fire in the courtyard of the Monastery: (2)
As of 2014-09-18 11:41 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    How do you remember the number of days in each month?











    Results (113 votes), past polls