Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83721
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.181 as permitted sender)
Message-ID: <54ED160B.9010908@gmail.com>
Date: Tue, 24 Feb 2015 16:23:39 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Thunderbird/31.4.0
MIME-Version: 1.0
To: =?UTF-8?B?UMOhZHJhaWMgQnJhZHk=?= <padraic.brady@gmail.com>
CC: Dmitry Stogov <dmitry@zend.com>, Yasuo Ohgaki <yohgaki@ohgaki.net>, 
 "internals@lists.php.net" <internals@lists.php.net>,
 Nikita Popov <nikitappv@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>	<54ECF605.7030506@gmail.com> <CALwr1GkU_mumpp-34Pp=gKwZzL3KVEW7JpEnqC5MMtcgM_qEbA@mail.gmail.com>
In-Reply-To: <CALwr1GkU_mumpp-34Pp=gKwZzL3KVEW7JpEnqC5MMtcgM_qEbA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] Script only include/require
From: smalyshev@gmail.com (Stanislav Malyshev)

Hi!

> Your example omitted the image validation step which would have

Ah, right, and if I name it .zip, it'd be zip validation, and if I name
it .pdf it'd be pdf validation, and if I name it .lol that would be LOL
validation. You'd have to manually validate every type in existence and
somehow invent validation for unknown ones. And it's not like I can't
also make it a valid GIF/PDF/whatever - that has been done already. So
you support this "security measure" by saying you can maybe plug the
hole using other measures. Maybe you can, maybe (more probably) not but
that does not redeem the insecurity of this "security measure".

> again. It's not very fair to create a scenario with a total lack of
> any security, and then ignore that your code's problem is that gaping
> hole and NOT the minor extension filter on the far end.

But that is *exactly* what this RFC is doing! The gaping security hole
is include $argv[1] and that's where it should be fixed, not introducing
temporary patches that prevent 1% of the scenarios and can be overridden
within minutes.

I don't see how it is not fair since the *only* scenario where it is
useful is when your code *already* has the gaping security hole,
otherwise this RFC has no utility as your includes are controlled by you
so they already are php files.

> indeed be preventable by his RFC. Please stick to what the RFC
> actually claims to do.

It claims to protect from file inclusion, by only allowing for include
to operate on strings which end in .php, and then banning such files
(ending in .php) from being handled by move_uploaded_file(). As I
demonstrated (and this is I suspect not the only option) this does not
actually offer any protection from LFI/RCE, as the end of the string
given to include and the file on disk do not have to be the same. In my
eyes, mechanism with such big BC break potential that is overridden in
so trivial manner has little value.

That even not considering upload doesn't even have to use
move_uploaded_file() either.
-- 
Stas Malyshev
smalyshev@gmail.com