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