Re: SQL injection, php and queueing multiple statement

Ivan Sergio Borgonovo wrote:
> On Sun, 13 Apr 2008 10:03:48 +0800
> Craig Ringer <craig(at)postnewspapers(dot)com(dot)au> wrote:
>
>
>> Your wrapper code can potentially do things like scan a string for
>> semicolons not enclosed in single or double quotes. The rule
>> probably has to be a little more complex than that, and has to
>> handle escaped quotes, but it might achieve what you want.
>>
>
> I think this logic is already somewhere in the driver or the pg
> engine. Whatever you write at the application level a) risk to be a
> duplication of part of the parser b) risk to be less smart than the
> parser itself and let slip something.
>
... in which case it sounds like you need to extend the Pg DB interface
to do what you want. It might be worth hacking together a proof of
concept and posting it to -hackers and the PHP interface maintainers,
along with a rationale for its inclusion.
>> I do think that for web scripting users it would be nice for DB
>> drivers to optionally throw an error if an execute call passes more
>> than one statement. Your problem would be that it'd need to be an
>> option that affects the whole app instance for it to achieve what
>> you want without developer action, and a global option like that
>> would potentially break 3rd party application code. The alternative
>> would be something like an executeSingle(...) call or a flag to
>> execute ... but that again just comes back to proper code review to
>> ensure it's used.
>>
>
> Why does it have to be "global", couldn't it be "by connection" or
> "by call to pg_query"?
>
Because you appear to be seeking something to protect against
programmers who do not follow coding guidelines, and that should help
even if code review processes fail to catch the problem. Were that not
the case you'd be able to use some of the other suggestions made here. I
quote:
>
> Yeah... but how can I effectively enforce the policy that ALL input
> will be passed through prepared statements?
>
> If I can't, and I doubt there is a system that will let me enforce
> that policy at a reasonable cost, why not providing a safety net that
> will at least raise the bar for the attacker at a very cheap cost?
>
> If programmers didn't make errors or errors where cheap to find there
>
How can you ensure that programmers will set the single statement flag
on each connection or set the flag on each query? If you can do those
things, you can also ensure that they just follow string handling rules,
use prepared statements where possible, require additional review of all
non-prepared-statement code, etc.

An app instance wide flag at least ensure that it's set once (say, in
PHP configuration) and *stays* set, no matter what stupid things
individual developers might do. For more general use a per-query and
per-connection flag would be useful, but it doesn't sound like it would
fit your specific needs unless you can hide the DB interface behind a
connection factory that you can ensure will always set the option. You
could also have a per-query-call flag to permit multiple statements;
it'd be something for library authors to use where required no matter
what the connection and global defaults were, ensuring they always got
correct behaviour. Such a flag would also be easy to scan for in
automated code review or in a revision control system hook script.

Also, as already noted there are ways to help you enforce the use of
prepared statements. If you have a decent code review process in place
then you probably have a `lint' script that's run as a first pass to
spot possible problems in the code. You can extend that script, and also
hook it into the revision control system so it emails you (or rejects
the commit) if somebody tries to commit code that looks worrying, like
non-prepared-query database interface calls. A simple annotation scheme
should permit you to ignore the legit ones, with unauthorized addition
of annotations punishable by cold coffee for a year.

There's probably an existing PHP lint script you can extend, so you
don't have to do the boring bits. A quick search suggests so.

Ideally you want a lint script or PHP extension that can also do static
and/or runtime analysis for user input `taint', like the Perl option of
the same name. A Google search for `php taint' suggests that there are
certainly efforts in that direction, though not being active in PHP it's
hard for me to tell how complete or useful they are.

In any case, I agree with you that a "single statement only" flag would
be nice in the DB interface, because as you say it's nice if all else
fails and will block a many of the most flexible types of SQL injection
attack. I just think that if it exists it needs to be opt-out, not
opt-in, to be significantly effective as a defense against other
programming errors.

--
Craig Ringer