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
[download]

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

Alex / talexb / Toronto

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

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.

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!
[download]

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.

Jenda


Support Denmark! Defend the free world!

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        

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
[download]

$ 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
[download]

bad_input() if($cityname !~ /^[a-zA-Z .,]+$/);
[download]

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.

The Open Web Application Security Project (OWASP) project has a tool called OWASP SQLiX that fits the description. It also happens to be written in Perl (by Cedric Cochin). Download here. cedri.cc states: "All content released under a Creative Commons License unless otherwise noted."

You should also read OWASP's Testing for SQL Injection article that includes a number of references to papers and tools touching the subject.

--
Andreas

Michael

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.

    @databases = qw ( CSV: DBM: );
[download]

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.

my $evil = join '', map chr, 0..255;
[download]