WP-A: A New URL Shortener

[Chris Angelico]

On Sat, Mar 26, 2016 at 9:25 AM, Thomas 'PointedEars' Lahn
<PointedEars at web.de> wrote:
> Chris Angelico wrote:
>
>> On Sat, Mar 26, 2016 at 8:28 AM, Thomas 'PointedEars' Lahn
>> <PointedEars at web.de> wrote:
>>> Chris Angelico wrote:
>>>> […] Thomas 'PointedEars' Lahn […] wrote:
>>>>> Chris Angelico wrote:
>>>>>> […] Thomas 'PointedEars' Lahn […] wrote:
>>>>>>> Daniel Wilcox wrote:
>>>>>>>> Cool thanks, highly recommended to use an ORM to deter easy SQL
>>>>>>>> injections.
>>>>>>> That is to crack a nut with a sledgehammer.  SQL injection can be
>>>>>>> easily and more efficiently prevented with prepared statements.  […]
>>>>>> You don't even need prepared statements. All you need is parameterized
>>>>>> queries.
>>>>> A prepared statement in this context uses a parameterized query.
>>>>>
>>>>>
>>>
> <https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet#Defense_Option_1:_Prepared_Statements_.28Parameterized_Queries.29>
>>>>
>>>> I know what a prepared statement is. And I know that they are
>>>> effective. However they are overkill - as I said, you merely need
>>>> parameterization.
>>>
>>> Then enlighten me, please: How is “parameterization” or a “parameterized
>>> query”, as *you* understand it, different from a prepared statement?
>>
>> This is a prepared statement:
>>
>> http://www.postgresql.org/docs/current/static/sql-prepare.html
>>
>> You use a special "PREPARE" query to create *and store* a half-run
>> query,
>
> [Having written a database layer (in PHP) myself, I know what a prepared
> statement is, thank you very much.]
>
> Your statement is incorrect both for MySQL (which I know) and PostgreSQL
> (AIUI your reference): The query is processed and stored of course, but it
> is definitely _not_ *run* before the EXECUTE command is issued for it.

It legal to partly perform the query. DB2 can retain a "query plan"
which consists of the exact paths it will follow. So I stand by my
statement.

>> Back in the 1990s, I had the option of actually *compiling* my SQL queries
>> as part of my C code, which would prepare all the queries for future
>> execution. It is completely different from the dynamic parameterized
>> queries that most people use.
>
> Interesting, but irrelevant.

Not really, no. It was a form of PREPARE/EXECUTE that hid the PREPARE
behind a convenient syntax, and then replaced it with EXECUTE for the
run-time.

>> Parameterization is a more general concept which prepared statements
>> invariably use, but which general code need not use.
>
> If it is to be safe from SQL injection, it better uses a parameterized query
> as it is *commonly* understood.

Why? All it needs to do is guarantee that no user data can affect the
query. There are myriad ways to do that, and prepared statements are
only one of them. Even parameterization is not strictly necessary, as
long as the escaping is perfect. (Though that does have its own
consequences, and is not recommended.)

>> A Python database connector could choose to PREPARE/EXECUTE for every
>> query it's given,
>
> That would not be wise, given that not every query contains variable
> parameters.

Irrelevant. It would still be legal. Do you understand the difference
between what is legal according to a specification and what is
actually worth doing? In an explanation of what a parameterized query
is, I would expect to be discussing what is semantically and
functionally valid, not which optimizations are worth doing.

>> or it could choose to escape all the parameters and embed them,
>
> Recommended against, and not a parameterized query at all.

Not once it reaches the underlying database, but it is parameterized
in the source code. It's not possible for an external attacker to get
past the escaping, if it is done correctly. It is still legal,
however, it is indeed not recommended.

>> or it could (if it's using a decent database back-end like PostgreSQL)
>> simply send the query and its associated parameters as-is. Only one of
>> these options is a "prepared statement".
>
> Maybe.  I do not know enough about PostgreSQL and its “[sending] the query
> and its associated parameters as-is” yet to confirm or deny this.

The wire protocol supports this.

>> All three are "parameterized queries", at least from the POV of Python
>> code.
>
> You are mistaken, then: Of the three kinds of “parameterized queries” as
> *you* understand them (OWASP and I beg to differ), at least one of them
> that is not used in a prepared statement is *insufficient to prevent SQL
> injection*.

Okay, then. Please explain how this code is vulnerable to SQL injection:

conn = some_db_module.connect("")
cur = conn.cursor()
data = input("Enter a value: ")
cur.execute("insert into some_table (some_column) values (%s)", (input,))
conn.commit()

Do you need to know the implementation of cur.execute to be able to
say whether this is safe, or can you be confident that, short of
really blatantly obvious bugs in the database connector, this is
genuinely a parameterized query? I posit that the latter is the case.
MY CODE has a query with a parameter. After that, it's not my problem;
attacks on lower-level services are always possible, but aren't called
"SQL injection".

> The second kind also moves tasks to the programming language that are better
> done by the DBMS; the program code should database-agnostic (in the best
> case, for mockup testing, even oblivious of the database).
>
> Which is why programming languages have come to support prepared statements,
> and why OWASP recommends to use either them or stored procedures.

What would you change in the above code? Does it really need prepared
statements or stored procedures? Surely the recommendation of Python
is to keep things simple and expressive?

ChrisA