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ä
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
