> On 24 Jun 2021, at 14:14, Scott Arciszewski <sc...@paragonie.com> wrote:
>
> On Thu, Jun 24, 2021 at 2:10 AM Stephen Reay <php-li...@koalephant.com> wrote:
>> Hi Scott,
>>
>> I wrote that example where an integer could be dangerous.
>
> I don't disagree that it's an example where an integer could be dangerous.
>
> Danger is too broad to have a meaningful discussion about. You can, of
> course, always do dangerous things if you're determined or creative
> enough.
>
>> So firstly - just to clarify, because some replies seemed to be confused on
>> the topic, as was literally mentioned in the original comment, it is
>> definitely not correct behaviour - it’s a developer mistake that might work
>> some of the time, and thus go unnoticed in testing. If you can show me a
>> developer who’s never inadvertently passed the wrong parameter in some
>> condition, I’ll show you an imaginary developer.
>
> Agreed.
>
>> Additionally - pointing out that this is a "developer error” doesn’t help
>> your case. Using non-parameterised queries should already be a “developer
>> error” for anyone who can walk and breathe at the same time - and yet that
>> usage is being actively encouraged if this function supports integers. I’ve
>> still seen zero responses about legitimate reasons this needs to support
>> integers - giving people a shitty way to build an IN() clause is not
>> legitimate. Parameterisation exists, and works.
>
> I don't have a strong opinion on this.
>
>> I don’t even understand why you mentioned prepared statements (I guess you
>> meant using parameterised queries?) - the column name inherently can’t be
>> parameterised - hence having to use a string substitution in the query.
>
> They're effectively synonyms.
I mean, they’re not - you can prepare a statement with zero parameters in it.
But my point was why you even mentioned parameterised queries OR prepared
statements at all. You cannot parameterise the column name, hence the entire
reason this RFC exists. The entire point is to make sure that the bits that you
_need_ to insert variables, are something the developer wrote, not some input
from the user.
If that isn’t the purpose of the RFC, then it describes its actual goal really
poorly.
>
>> That part was weird and confusing, but not as odd as your claim that
>> altering the query, so that it causes the where clause to become moot, is
>> not an SQL Injection? REALLY? That’s your claim?
>
> Yes, let me try to explain this more clearly.
>
> If you can inject code, it's a code injection vulnerability. SQL
> injection is a specific type of code injection vulnerability. If you
> can trick the SQL into doing something invalid *without* injecting
> code, and you call it "SQL injection", that's a category error.
>
> Supplying an integer in the place of a column name is a bug. The bug
> can even be dangerous. The bug can EVEN be a security problem for the
> system.
>
> But calling it SQL injection is categorically incorrect. Changing the
> behavior of a SQL command **without injecting additional code** is not
> SQL injection.
>
> If we're trying to stop all forms of misuse and insecurity that can
> result from string concatenation, cool. But be very clear about that.
>
>> I did a little research, and it turns out Wikipedia
>> (https://en.wikipedia.org/wiki/SQL_injection#Technical_implementations),
>> Cloudflare
>> (https://www.cloudflare.com/en-au/learning/security/threats/sql-injection/),
>> and OWASP (https://owasp.org/www-community/attacks/SQL_Injection#example-2)
>> all have examples with a `1=1` type query manipulation. Do you want to write
>> and tell them that they’re all wrong, or should I ask them to call you?
>
> If you **inject a `1=1` clause where one didn't exist before**, that's
> injection. Notice the introduction of an OR operator in the examples
> you cited.
Please, explain to us all, how `where foo=‘bar’ OR 1=1` is functionally
different than `where 123=123` because the developer screwed the pooch in some
specific condition and passed the ID twice, rather than the column name and the
id, respectively.
>
> My classification of the original example as "Not Injection" has
> nothing to do with the fact that numbers are being compared with
> numbers. Rather, there was no code injection.
>
>> Also, while researching the specifics of what is considered an “SQL
>> Injection” I came across an article, that talks specifically about the
>> dangers of allowing user input (i.e. the thing `is_trusted` is meant to
>> prevent) as a column or table identifier. It’s from this little
>> organisation, you may have heard of them: “Paragon Initiative”
>> (https://paragonie.com/blog/2015/05/preventing-sql-injection-in-php-applications-easy-and-definitive-guide).
> Snark is unnecessary.
You call it snark, I call it ironic hypocrisy.
>
>> I would absolutely make use of a function that tells me if the string given
>> is in fact from something controlled by the developer. But once that same
>> string can also include input from the request or the environment or
>> whatever by nature of integers, the function becomes useless for the stated
>> purpose.
>
> Why not two functions then?
>
> - is_noble_string() -- more restrictive
> - is_noble() -- YOLO
I mean sure, if someone wants to vote on the two choices or whatever, I’ll go
get the popcorn, but I still haven’t seen any answers that make sense to this
question: Why integers at all, though?
>
>> Cheers
>>
>> Stephen
>>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
