Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114963
MIME-Version: 1.0
In-Reply-To: <CAFv4g+GsCFv6XvM4DeUK6L8Or4c0inwz8x=jDQAV=r-S4roOtQ@mail.gmail.com>
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> <CAFv4g+GsCFv6XvM4DeUK6L8Or4c0inwz8x=jDQAV=r-S4roOtQ@mail.gmail.com>
Date: Fri, 18 Jun 2021 19:48:28 +0200
Message-ID: <CAOwTYAvPtTdQ34XmywxZrpUHuMsCER5cMO-X3oVaOFo6DZXA1Q@mail.gmail.com>
To: Craig Francis <craig@craigfrancis.co.uk>
Cc: Bruce Weirdan <weirdan@gmail.com>, Guilliam Xavier <guilliam.xavier@gmail.com>, 
	PHP Development <internals@lists.php.net>, Pierre <pierre-php@processus.org>
Content-Type: multipart/alternative; boundary="0000000000006c87e905c50dee3d"
Subject: Re: [PHP-DEV] [RFC] is_literal
From: krakjoe@gmail.com (Joe Watkins)

--0000000000006c87e905c50dee3d
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Just to explain this tracking of longs.

We can track strings because there's space to do that in GC info which
recounted types all have.

Where things aren't recounted there is no usable space on the value to
store the literal flag without disabling some internal assumptions about
type flags.

We have explored this, it wouldn't be an acceptable implementation detail.

Cheers
Joe

On Friday, 18 June 2021, Craig Francis <craig@craigfrancis.co.uk> wrote:

> 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 fro=
m
> 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 lo=
gical
> flaws from the Developer; what you=E2=80=99re describing is a different i=
ssue. This
> RFC is to help prevent Injection Vulnerabilities, and it can=E2=80=99t be=
 an
> Injection Vulnerability because the variable doesn=E2=80=99t contain comm=
ands, 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 els=
e 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 somethin=
g
> 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 =
same
> thing, and get that same outcome.
>
>
>
> Overall I think allowing ints in literal concatenation without tainting t=
he
> > result as non-literal is a mistake.
>
>
>
> To counter this, supporting integers makes adoption easier, and it=E2=80=
=99s not
> causing any security issues.
>
> I prefer something that more developers/systems will be able to easily us=
e,
> 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 i=
t, 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 inte=
gers or no
> integers at all. And no integers really isn=E2=80=99t feasible for most d=
evelopers.
>
> Craig
>

--0000000000006c87e905c50dee3d--