Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100655
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain me.com designates 17.142.194.250 as permitted sender)
Date: Fri, 15 Sep 2017 23:29:28 +0200
To: Sara Golemon <pollita@php.net>, Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: Marco Pivetta <ocramius@gmail.com>, PHP internals <internals@lists.php.net>
Message-ID: <92ef8ef2-bbf3-4f9e-9984-460cfcd07492@Spark>
In-reply-to:
 <CAGa2bXbbEqRJmcLF7G-UMzOZyu9FJvOcNuXJyp+20C+YkQhHMA@mail.gmail.com>
References: <097578bf-ab74-44cf-a465-dc6fdd50930f@Spark>
 <CADyq6sKQ5RqOQvcRK_dSnu+wtHBqwMkoH_Vhxw8NZCi2tk7YPQ@mail.gmail.com>
 <7d703ad8-5596-44e7-95dc-23c2cc058408@Spark>
 <CAESVnVqi6O7UNmX=jA3nwCLDYXqrPRwZKWYsuoeitjhAX0AGfQ@mail.gmail.com>
 <CAGa2bXbbEqRJmcLF7G-UMzOZyu9FJvOcNuXJyp+20C+YkQhHMA@mail.gmail.com>
MIME-version: 1.0
Content-type: multipart/alternative; boundary=59bc46a2_327b23c6_7ae1
Subject: Re: [PHP-DEV] [RFC] Deprecate the extract function in PHP 7.3
From: ilija.tovilo@me.com

--59bc46a2_327b23c6_7ae1
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

Hi Yasuo

These sound good to me=21
There=E2=80=99s still a smaller vulnerability of defining variables befor=
ehand:

=60=60=60
=24data =3D =5B'hasAccess' =3D> true=5D;

export(=24data);

if (=24user->isAdmin()) =7B
=C2=A0 =C2=A0 =24hasAccess =3D true;
=7D

if (isset(=24hasAccess) && =24hasAccess =3D=3D=3D true) =7B
=C2=A0 =C2=A0 print 'Bingo';
=7D
=60=60=60

but code like this should be rather rare.

Regards


On 15 Sep 2017, 23:17 +0200, Yasuo Ohgaki <yohgaki=40ohgaki.net>, wrote:
> Hi all,
>
> On Sat, Sep 16, 2017 at 2:50 AM, Sara Golemon <pollita=40php.net> wrote=
:
>
> > On =46ri, Sep 15, 2017 at 1:35 PM, <ilija.tovilo=40me.com> wrote:
> > > The =60extract=60 function takes an associative array and
> > > puts it into the local symbol table.
> > > http://php.net/manual/en/function.extract.php
> > >
> > > I seriously doubt the usefulness of this function,
> > > especially looking at the potential risks. The fact
> > > that overwriting the local variables is the default
> > > behaviour doesn=E2=80=99t make it any better. I suggest
> > > deprecating it in PHP 7.3 and removing it in 8.
> > >
> > Preface: I despise extract() as well. It's breaks assumptions for
> > both the developer and the runtime. I save some of my frowniest of
> > faces for extract().
> >
> > That said...
> >
> > > I can see it=E2=80=99s usefulness in this case.
> > > But wouldn=E2=80=99t it be better to implement this by hand
> > > in these rare cases (it=E2=80=99s 3 lines of code) instead of
> > > encouraging the pollution of the symbol table by
> > > unknown input=3F It=E2=80=99s also clearer since people who
> > > don=E2=80=99t know the =60extract=60 function probably don=E2=80=99=
t
> > > expect it to mutate the local symbol table.
> > >
> > Let's be clear on what that looks like: foreach (=24data as =24key =3D=

> > =24value) =7B =24=24key =3D =24value; =7D
> >
> > This is SO MUCH WORSE for several reasons, no least of all what
> > happens when =24data contains keys named 'data', 'key', or 'value'.
> >
> > I'd like to kill extract(), but it does have a reason for being, and =
I
> > couldn't in any good conscience support removing it without a
> > replacement that's at least marginally better.
> >
>
> The security issue regarding extract() are:
> - Unintended variable creation. e.g. =24admin=5Fflag =3D ture
> - Unintended variable modification. e.g. =24admin=5Fflag =3D ture
>
> Instead of removing/deprecating extract(),
> - Add EXTR=5FEXCEPTION flag that throws exception for overwriting.
> (There are many flags, but no error/exception flag. This isn't good)
> - Require prefix. (User may still set '' as prefix)
>
> We may do:
> - Add EXTR=5FEXCEPTION flag in 7.3
> - Make all three parameters required parameter set EXTR=5FECEPTION a de=
fault
> flag in 8.0
>
> With this way, we'll have better compatibility with older PHP and bette=
r
> security with extract().
>
> https://github.com/php/php-src/blob/master/ext/standard/array.c=23L2459=

> Since it checks valid flags, we wouldn't have compatibility for current=

> versions unless
> released versions code is modified for it.
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohgaki=40ohgaki.net

--59bc46a2_327b23c6_7ae1--