On Thu, Jun 24, 2021, at 11:18 AM, Scott Arciszewski wrote:
> On Thu, Jun 24, 2021 at 5:22 AM Stephen Reay <php-li...@koalephant.com> wrote:

> 1. I never claimed that it wasn't a bug.
> 2. I never claimed it wasn't impactful.
> 3. I never claimed it wasn't security-affecting.
> 
> I've simply said that this isn't an example of SQL Injection, because
> nothing is being *injected*.
> 
> > >
> > > 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.
> 
> In my original email, I said. "Outside the chr()/pack()/sprintf()/etc.
> technique demonstrated above, there's no risk of injection inherent to
> concatenating a trusted string with an untrusted integer." You brought
> up a high-severity logic bug that could happen if someone concatenates
> variables with wild abandon, but it's still not an example of
> **injection**.
> 
> If you're going to call someone a hypocrite (which is a needless
> personal attack), take care that you're actually correct when you do
> so.

First, I think it's best if we all stop the definitional meta-debate, it's not 
really helpful.

Second, FFS people, use the Reply to List feature.  I've gotten double copies 
of the last 30 messages in this thread.

Third, the core point here is:

1. A literal string comes from the developer, in source.
2. Thus a literal string is safe against SQL security errors.
3. An integer cannot contain SQL syntax.
4. Thus a literal string concat with an integer cannot contain SQL syntax 
errors.
5. Thus a literal string concat with an integer is as safe as a literal string.
6. Thus we can still call a literal string contact with an integer a literal 
string.

That's the logic that leads to "a string literal contact an integer should 
still return true from is_literal()."  It's the logic that most users would go 
through upon seeing that, and act on that conclusion.

However, it's been demonstrated that step 4 is NOT true, and a literal string 
contact with an int can lead to a security vulnerability (whether it's 
technically an SQL injection per se is not relevant).

Thus, I would argue that "literal string concat int --> literal string" is 
incorrect and should be removed from the RFC, as it is misleading.  That it may 
hurt adoption is irrelevant, as creating a false sense of security is vastly 
worse than slower adoption.

--Larry Garfield

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to