Sent from my iPhone
> On 23 Jun 2021, at 03:08, Lauri Kenttä <lauri.ken...@gmail.com> wrote:
>
> On 2021-06-22 21:38, Stephen Reay wrote:
>>>> On 22 Jun 2021, at 21:38, Craig Francis <cr...@craigfrancis.co.uk> wrote:
>>> If you can point me to an example where including integers in this has
>>> introduced a security vulnerability then please do, and I mean it, that’s
>>> what this process is for, I genuinely want people to come forward with them
>>> so we can refine this.
>> It took me about a minute to think of this:
>> "select * from customer_purchases where {$column} = :value”.
>> The developer inadvertently passes the same “trusted value” in as the
>> `$column` substitute and the value parameter. It must be safe because
>> we ran it through `is_trusted`!
>> The query now executes as:
>> "select * from customer_purchases where 12345 = 12345”
>> You cannot magically make all dynamically generated queries safe -
>> they tried that about a quarter of a century ago. Hint: it did not end
>> well - and explicitly allowing some user input is just mind boggling
>> given the stated goals.
>
>
> Your example is interesting and kind of valid. Looks like there is a bug in
> the code: $column is sometimes an user-defined integer and sometimes a valid
> literal column name, clearly this should not happen. (If it was supposed to
> be integer, you would pass it as a parameter just like :value, right?)
>
> Is this RFC about preventing bugs (accidentally used wrong variable) or
> preventing bad code (user input intentional but used the wrong way)? I
> thought it was more the about bad code. Bugs can live even in literal queries
> as well as outside queries, where is_literal/is_trusted can't reach them.
>
> Another line of thought: One possible approach would be to accept only
> explicit integer casts (sprintf %d and %u, and a new function like
> implode_ints() and/or strval_int()) and otherwise only accept strings. This
> would avoid the accidental case where $x is supposed to be a trusted string
> but is an untrusted integer instead, like the given example.
>
> --
> Lauri Kenttä
>
Yes it very deliberately shows a case where developer error can lead to
incorrect behaviour, because of a bug. I don’t think it’s a stretch to say that
not validating the dynamic elements used to generate a query would also be
considered a bug caused by developer error.
Given that this is meant to be a security feature, an approach of “well this is
probably fine” seems like a pretty cavalier approach to start from, especially
given that there are existing solutions to prevent injections of integer values.
Rather than people keep repeating “integers aren’t injections”, perhaps someone
can show just one example why you’d need to use a literal integer as an
identifier in sql (ie a table name etc) and thus can’t use parameterisation.
