Beefy Boxes and Bandwidth Generously Provided by pair Networks
The stupid question is the question not asked
 
PerlMonks  

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

by stvn (Monsignor)
on Jan 09, 2008 at 21:14 UTC ( #661506=note: print w/ replies, xml ) Need Help??


in reply to Re: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
in thread Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite

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


Comment on Re^2: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
Re^3: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by siracusa (Friar) on Jan 10, 2008 at 14:10 UTC
    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 :)

Re^3: Preventing SQL injection attacks: Placeholders are enough for MySQL, Postgresql and SQLite
by Jenda (Abbot) on Jan 11, 2008 at 00:21 UTC

    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.

      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.
      1. How I use my database is my business, and in some of my business use cases it makes more sense to treat it like a dumb object store. For that I use DBIx::Class.
      2. Why the f*ck would I want your old dancing shoes anyway?

      Now, before you make assumptions, I suggest you actually take a look at the DBIx::Class code and the SQL queries it does generate (you can use DBIx::Class::QueryLog for that). To start with DBIx::Class allows you an extremely high degree of control over when and how much of the database it queries. Second, the many contributors to the project have made sure that the SQL it does generate is both optimized and smart (after all these people are not idiots they know how databases work very well).

      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.

      If that is all you want then why do you care what I do? I am not going to tell you that what you doing is inefficient and stupid, cause I assume that you are smart enough to do that is most appropriate for your business use case. Remember, this is Perl, the land of TIMTOWTDI, you do it your way and I will do it mine. But please, please, please don't talk shit about stuff you have no clue about (*cough* DBIx::Class *cough*).

      -stvn

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://661506]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others taking refuge in the Monastery: (8)
As of 2014-12-18 12:08 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Is guessing a good strategy for surviving in the IT business?





    Results (51 votes), past polls