Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115124
User-Agent: Cyrus-JMAP/3.5.0-alpha0-530-gd0c265785f-fm-20210616.002-gd0c26578
Mime-Version: 1.0
Message-ID: <a907cde7-9100-4c5f-917c-2c332aa3c67f@www.fastmail.com>
In-Reply-To: 
 <CAKws9z0P1K8FQ3PqoZqh6zm995S60siTi25qWUMo7=ZJ=DAv0w@mail.gmail.com>
References: 
 <CAFv4g+Gv1qQ2Gtx2Qwo0Dx2Cb_gOXJ=rm7rVy+Jo89PmD-3jnA@mail.gmail.com>
 <CAEMvS4GRaqMBPn7ocBX-XU3jzrtpe+9uGfYpG10qc_iKZY80UA@mail.gmail.com>
 <03f7955c-69a8-4841-9245-449d7851e207@www.fastmail.com>
 <CAFv4g+EaT309KDWz63j3S7n8E58czEWTKUEw9hAvgt7s6R9eKQ@mail.gmail.com>
 <CAKws9z02yCZTb2_mw-5xUO5-QLc4jbOUcBRfJ8nFSNPP1rgO3g@mail.gmail.com>
 <CA+hqhpfUiOMLDZyXTqP+mm9jjG6v+K94Ug7K_zvuuZus_XhOaQ@mail.gmail.com>
 <CAKws9z34nTT0_CXoo4Yq7zO2CZX622has23xmfHbWjfaDrgLqA@mail.gmail.com>
 <CA+hqhpdR=W8nbWy8UBgLtKdvb_KCss5mpzturS67HhqRj4q6BQ@mail.gmail.com>
 <CAKws9z1=6fmCxJWYVfN+xKT+u=_ZTq0m7C0nZPT6LB5scbfpXQ@mail.gmail.com>
 <95D16F2E-E9DD-4964-A0E2-62E1FB0D976B@koalephant.com>
 <CAKws9z0CbyjeKas=DNKi0ScdUxt8CnvNOLymXpWvQTaJtB0xkw@mail.gmail.com>
 <0427D84F-762D-48D3-B4D1-FD4E405274C4@koalephant.com>
 <CAKws9z0P1K8FQ3PqoZqh6zm995S60siTi25qWUMo7=ZJ=DAv0w@mail.gmail.com>
Date: Thu, 24 Jun 2021 11:57:39 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [RFC] Name issue - is_literal/is_trusted
From: larry@garfieldtech.com ("Larry Garfield")

On Thu, Jun 24, 2021, at 11:18 AM, Scott Arciszewski wrote:
> On Thu, Jun 24, 2021 at 5:22 AM Stephen Reay <php-lists@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.
>=20
> I've simply said that this isn't an example of SQL Injection, because
> nothing is being *injected*.
>=20
> > >
> > > 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 =E2=
=80=9CSQL Injection=E2=80=9D I came across an article, that talks specif=
ically about the dangers of allowing user input (i.e. the thing `is_trus=
ted` is meant to prevent) as a column or table identifier. It=E2=80=99s =
from this little organisation, you may have heard of them: =E2=80=9CPara=
gon Initiative=E2=80=9D (https://paragonie.com/blog/2015/05/preventing-s=
ql-injection-in-php-applications-easy-and-definitive-guide).
> > > Snark is unnecessary.
> >
> > You call it snark, I call it ironic hypocrisy.
>=20
> 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**.
>=20
> 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 c=
opies 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 synta=
x 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 lit=
eral string.

That's the logic that leads to "a string literal contact an integer shou=
ld 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 s=
tring 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.  T=
hat it may hurt adoption is irrelevant, as creating a false sense of sec=
urity is vastly worse than slower adoption.

--Larry Garfield