Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83729
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.53 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <54ED1CE2.9060903@gmail.com>
References: <CAGa2bXa=LHMT+wdSM_xbHDo1901QDSFa-KotjQckNwVX9wUEUQ@mail.gmail.com>
 <CAGa2bXZ0VCC4ZN0=YU8ZE9LXm8m9j-RJnmYOhd5qrENoa1HimQ@mail.gmail.com>
 <CA+9eiLvV-wY-6BC5FHZM=uMAWm4Ny4Ej_m2F31o6DNGacpfJPQ@mail.gmail.com>
 <CALwr1Gmmy06_4ZgsvUhcf5m7g06vK2s18qmxKBv1gu+syrsbQA@mail.gmail.com>
 <54ECD4E3.9040705@gmail.com> <CALwr1GkY_8TN-HwXnRNL0-1oRfQPJvz0_vyJv_ojUTVsFbkNTQ@mail.gmail.com>
 <CAGa2bXZV1FOaW5mJqx2yw7WQz0prp1zfEgdEw99StbWPgDFUwg@mail.gmail.com>
 <54ECFAA8.4020305@gmail.com> <CAGa2bXZ373PGrJtpmgLje3obzGcXQs5sND=8TzyuKac+hZ--Fg@mail.gmail.com>
 <54ED085C.8010901@gmail.com> <CAGa2bXYDGXu3coifbAdLUqE=1qPEJb4RnC_-vWctcrFirc-S6Q@mail.gmail.com>
 <54ED1CE2.9060903@gmail.com>
Date: Wed, 25 Feb 2015 09:59:08 +0900
Message-ID: <CAGa2bXaKG3p02Y0X22NP7W=qdO7N4pq8ikqNoF7xm0vUBzMmbg@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: =?UTF-8?Q?P=C3=A1draic_Brady?= <padraic.brady@gmail.com>, 
	"internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11392386f4f602050fdf26ab
Subject: Re: [PHP-DEV] [RFC] Script only include/require
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a11392386f4f602050fdf26ab
Content-Type: text/plain; charset=UTF-8

Hi Stas,

On Wed, Feb 25, 2015 at 9:52 AM, Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> >  - require/include only includes ".php" ".phar" by default.
>
> This is not true. As I repeatedly point out, your change only requires
> that the string passed to include would end in .php, but string passed
> to include and filename on filesystem are very different things, they do
> not have to be the same at all!
>
> >  - move_uplaoded_file() only move files other than ".php", ".phar"
> > extension by default.
> >
> > This makes "script inclusion" attack much harder. In fact, this RFC
> > makes almost
> > impossible attackers to take advantage of PHP's embed everything nature.
>
> I am at loss how you can claim it is "impossible" after I just
> demonstrated how it is possible.
>
> > For users do not need these protections, they can simply add one liner.
> >
> > ini_set('zend.script_extensions', '');
> >
> > This piece of code is executed for all PHP version without any errors.
>
> You sincerely do not understand why breaking all code that uses non-php
> extensions is a problem and that editing existing production code's
> entry points is by no way "simple"? That finding all extensions that may
> be used for files in a complex software is very far from "simple"?
>
> > Unlike previous RFC, this RFC patch does not care "contents" of file, but
> > only cares file "extensions". However, it works well as described before.
>
> It doesn't work well - in fact, it does not work at all, as I just
> demonstrated, exactly because extension of the actual file and what
> you're checking are different things.
>
> > Even with this RFC, users can shoot their own foot and PHP is much easier
> > to do it wrong compare to other languages.
>
> These platitudes do not change the simple fact that your RFC does not do
> what it claims to do, security-wise.
>
> > However, if users use new PHP by default setting, PHP is as safe as
> > other languages against script inclusion/upload attacks. Since we have
> > move_uploaded_file() protection, PHP becomes safer than others.
>
> No, it does not become safer, because if you have code that allows
> script inclusion injection (and if you do, it's your fault, not PHP's)
> it is trivial to exploit it even with this RFC.
>
> >  - require/include cannot load script other than ".php"/".phar".
>
> As I showed, this is not true. I am kind of tired of repeating the same
> thing again and again and being completely ignored, so unless there's
> some new argument I'll stop repeating myself now.


I think you misunderstood the RFC.
You seems forgetting about "allow_url_include=Off" by default.

Are you saying current PHP allows
include('zip://...') or include('input://...')?

Then this is serious bug. I'll fix it also.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--001a11392386f4f602050fdf26ab--