Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83537
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.48 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <54EA5162.6060408@gmail.com>
References: <CAGa2bXZ_Q4Ytz5HgcmBoKNPzUaCh6vLETkyyd-jVsR9df60RPg@mail.gmail.com>
 <54EA5162.6060408@gmail.com>
Date: Mon, 23 Feb 2015 12:34:07 +0900
Message-ID: <CAGa2bXaCTxXqL9W85=eaiq=1Rb9tMJSkRqZ8fH=veReUZYHciw@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11c125ec843d64050fb915be
Subject: Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a11c125ec843d64050fb915be
Content-Type: text/plain; charset=UTF-8

Hi Stas,

On Mon, Feb 23, 2015 at 7:00 AM, Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> > I think this will be the final discussion before vote.
> > This RFC is to make PHP stronger against script inclusion attacks just
> like
> > other languages.
> >
> > https://wiki.php.net/rfc/script_only_include
>
> I still think this RFC takes a wrong road for the following reasons:
>
> 1. Having any code in your app that allows to run include on
> user-controlled files (I'm not talking about filtered cases but user
> data controlling the path) is insecure and can not be made secure. It
> should just never be done. Trying to find workarounds for this is like
> safe_mode - good idea in theory, leads to worse security in practice.
>

This is mitigation proposal against script inclusions. The difference is
clear
by statistics.

Because this is mitigation, it does not aims to be a perfect solution. It
aims
to make PHP as secure as other languages.

I think system admins feel more comfortable with this change, too.
They know PHP programs are very weak against script inclusion attacks
compare to other languages.


>
> 2. Default configuration would break tons of PHP scripts with extensions
> other than .php (very frequent case). The BC break potential of this is
> very big as it modifies core functionality.
>

Compatibility can be provided by one liner.

ini_set('zend.script_extensions', '.php .phar .inc .phtml .php4 .php5');

ini_set() does not emit any errors for non existing INIs.


3. Prohibiting phar uploads would also be a bc break, but more
> importantly, there still probably are ways to work around this by using
> phar files with extension different than .phar and then asking to
> include files within that phar file. As long as the eventual path would
> end in .php, your code would allow it.
>

Security is trade off relation, so I think this change acceptable trade off
to disable "script inclusion" (executing attacker programs).

Users can move uploaded files safely without move_uploaded_file() now.
I just made use of it to provide another mitigation, since script only
include
cannot be mitigation for uploading script files under docroot.

Also, the claim that move_upload_file() is obsolete is not based on
> anything as far as I can see. Why is it "obsolete"?


move_uplaoded_file() is needed for "register_globals". Attacker could
specify source files (i.e. in $_FILES) other than uploaded files with
"register_globals".

Current move_uploaded_file() checks source filename is really a
uploaded file's filename. It prevents moving other files, so it's not
completely useless but there is not real protections now because
values in $_FILES is safe now.

I know your point of view, but I hope you like this RFC.
Thank you for your comment. Your comments are very helpful to
come up with this RFC.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--001a11c125ec843d64050fb915be--