Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115102
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.100.0.2.22\))
In-Reply-To: <CAKws9z0CbyjeKas=DNKi0ScdUxt8CnvNOLymXpWvQTaJtB0xkw@mail.gmail.com>
Date: Thu, 24 Jun 2021 16:22:35 +0700
Cc: Bruce Weirdan <weirdan@gmail.com>,
 Craig Francis <craig@craigfrancis.co.uk>,
 Larry Garfield <larry@garfieldtech.com>,
 php internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <0427D84F-762D-48D3-B4D1-FD4E405274C4@koalephant.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>
To: Scott Arciszewski <scott@paragonie.com>
Subject: Re: [PHP-DEV] [RFC] Name issue - is_literal/is_trusted
From: php-lists@koalephant.com (Stephen Reay)



> On 24 Jun 2021, at 14:14, Scott Arciszewski <scott@paragonie.com> =
wrote:
>=20
> On Thu, Jun 24, 2021 at 2:10 AM Stephen Reay =
<php-lists@koalephant.com> wrote:
>> Hi Scott,
>>=20
>> I wrote that example where an integer could be dangerous.
>=20
> I don't disagree that it's an example where an integer could be =
dangerous.
>=20
> 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.
>=20
>> 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=E2=80=99s a =
developer mistake that might work some of the time, and thus go =
unnoticed in testing. If you can show me a developer who=E2=80=99s never =
inadvertently passed the wrong parameter in some condition, I=E2=80=99ll =
show you an imaginary developer.
>=20
> Agreed.
>=20
>> Additionally - pointing out that this is a "developer error=E2=80=9D =
doesn=E2=80=99t help your case. Using non-parameterised queries should =
already be a =E2=80=9Cdeveloper error=E2=80=9D 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=E2=80=99ve 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.
>=20
> I don't have a strong opinion on this.
>=20
>> I don=E2=80=99t even understand why you mentioned prepared statements =
(I guess you meant using parameterised queries?) - the column name =
inherently can=E2=80=99t be parameterised - hence having to use a string =
substitution in the query.
>=20
> They're effectively synonyms.

I mean, they=E2=80=99re not - you can prepare a statement with zero =
parameters in it.=20
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=E2=80=99t the purpose of the RFC, then it describes its =
actual goal really poorly.

>=20
>> 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=E2=80=99s your claim?
>=20
> Yes, let me try to explain this more clearly.
>=20
> 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.
>=20
> 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.
>=20
> But calling it SQL injection is categorically incorrect. Changing the
> behavior of a SQL command **without injecting additional code** is not
> SQL injection.
>=20
> 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.
>=20
>> 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=3D1` type query manipulation. Do you want to =
write and tell them that they=E2=80=99re all wrong, or should I ask them =
to call you?
>=20
> If you **inject a `1=3D1` 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=3D=E2=80=98bar=E2=80=99 OR =
1=3D1` is functionally different than `where 123=3D123` because the =
developer screwed the pooch in some specific condition and passed the ID =
twice, rather than the column name and the id, respectively.

>=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.
>=20
>> Also, while researching the specifics of what is considered an =E2=80=9C=
SQL Injection=E2=80=9D 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=E2=80=99s from =
this little organisation, you may have heard of them: =E2=80=9CParagon =
Initiative=E2=80=9D =
(https://paragonie.com/blog/2015/05/preventing-sql-injection-in-php-applic=
ations-easy-and-definitive-guide).
> Snark is unnecessary.

You call it snark, I call it ironic hypocrisy.

>=20
>> 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.
>=20
> Why not two functions then?
>=20
> - is_noble_string() -- more restrictive
> - is_noble() -- YOLO

I mean sure, if someone wants to vote on the two choices or whatever, =
I=E2=80=99ll go get the popcorn, but I still haven=E2=80=99t seen any =
answers that make sense to this question:  Why integers at all, though?=20=



>=20
>> Cheers
>>=20
>> Stephen
>>=20
>=20
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>=20