Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114962
MIME-Version: 1.0
References: <CAFv4g+F-7F7o6va5wCrojKe+Uysx2Ec3H4yV_mUZYEyNm3MkdQ@mail.gmail.com>
 <CAFv4g+GYwH-nQ8bLGbm=ThARghGUG5hMpw7W5SjMnSPMTom7+g@mail.gmail.com>
 <CAFv4g+GA9y62phk1TB8Z9EakpAXwmOa5pGxT1pdBVZwkCNxsVA@mail.gmail.com>
 <f9c70ab3-3218-6fc0-06b3-4041f3f78cd4@processus.org> <CAC5V6gi_U-ZHHbYj_MvOw_ifp5hCwnQt6+A3pmS==UDAKO74Yw@mail.gmail.com>
 <d221707f-7382-479e-6752-d9661f79a303@processus.org> <CAFv4g+FULOqtzJk20p6a_tBWZMOpLJEhyrwn95hGUTrWwOMC2A@mail.gmail.com>
 <CA+hqhpe9xf6=F0X0oeUw1mP9sotax0365gJoDbEqRb9e1YOP_w@mail.gmail.com>
In-Reply-To: <CA+hqhpe9xf6=F0X0oeUw1mP9sotax0365gJoDbEqRb9e1YOP_w@mail.gmail.com>
Date: Fri, 18 Jun 2021 18:22:56 +0100
Message-ID: <CAFv4g+GsCFv6XvM4DeUK6L8Or4c0inwz8x=jDQAV=r-S4roOtQ@mail.gmail.com>
To: Bruce Weirdan <weirdan@gmail.com>
Cc: Guilliam Xavier <guilliam.xavier@gmail.com>, PHP Development <internals@lists.php.net>, 
	Pierre <pierre-php@processus.org>
Content-Type: multipart/alternative; boundary="000000000000bba87905c50d93cc"
Subject: Re: [PHP-DEV] Re: [RFC] is_literal
From: craig@craigfrancis.co.uk (Craig Francis)

--000000000000bba87905c50d93cc
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, 18 Jun 2021 at 15:47, Bruce Weirdan <weirdan@gmail.com> wrote:

> One would be potential denial of service prevention (e.g. with enormous
> `LIMIT` value where only a limited set of ints was intended.
> [...]

Here you really *don't* want $allowed_ids to include user input.




The developer is writing this query to find those IDs and what the
developer asks for is what they get, the values inside that list came from
somewhere else and *that part earlier* is the bit with the unknown
vulnerability that allowed the value in there. I can=E2=80=99t address logi=
cal
flaws from the Developer; what you=E2=80=99re describing is a different iss=
ue. This
RFC is to help prevent Injection Vulnerabilities, and it can=E2=80=99t be a=
n
Injection Vulnerability because the variable doesn=E2=80=99t contain comman=
ds, it=E2=80=99s
only containing values. We cannot create anything entirely =E2=80=98safe=E2=
=80=99 there=E2=80=99s
no such thing as =E2=80=98safe code=E2=80=99, and neither I or anyone else =
can hope to
create a single RFC that can do that.

If we just use the original is_literal (without integers), the only
difference in your example will be that, when using integers for something
like this, the developer will need to use a Parameterised Query instead
(because we=E2=80=99d only be accepting literal strings) to do the exact sa=
me
thing, and get that same outcome.



Overall I think allowing ints in literal concatenation without tainting the
> result as non-literal is a mistake.



To counter this, supporting integers makes adoption easier, and it=E2=80=99=
s not
causing any security issues.

I prefer something that more developers/systems will be able to easily use,
especially when updating existing code.




> BTW, Psalm already distinguishes `literal-int` from `int`
>


Psalm is its own engine - it=E2=80=99s looking at the code, not running it,=
 so it=E2=80=99s
not negatively impacting performance. However PHP itself cannot do this,
integers cannot include a flag to state if they came from source code,
because adding would have too much of a performance impact.

Or to quote Joe directly "We can't really do that, non reference counted
types are stack allocated, ie the zval on either side of a call boundary
are unique."

We can=E2=80=99t split integers into two groups. It=E2=80=99s accept intege=
rs or no
integers at all. And no integers really isn=E2=80=99t feasible for most dev=
elopers.

Craig

--000000000000bba87905c50d93cc--