Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:84085
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.175 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <54F149BC.70107@gmail.com>
References: <CAGa2bXYf7Bi1QPya96t0Z3cQkTWBCOG2p2gCLZSUTY=_gNp_kw@mail.gmail.com>
 <54F0540C.1040807@gmail.com> <CAGa2bXbgBT6A3_-ihQyNrgE-ayVX+6UtdtJOX4=xQYYz+WFFwQ@mail.gmail.com>
 <54F149BC.70107@gmail.com>
Date: Sun, 1 Mar 2015 08:41:50 +0900
Message-ID: <CAGa2bXYVgR7QE8ktX8Hp68VdqcwVZQjvLNB1FM1zkfYYb=371w@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: Rowan Collins <rowan.collins@gmail.com>, 
	"internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a1143254ecffcf705102e8935
Subject: Re: [PHP-DEV] [RFC][DISCUSSION] Remove allow_url_include INI
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a1143254ecffcf705102e8935
Content-Type: text/plain; charset=UTF-8

Hi Stas,

On Sat, Feb 28, 2015 at 1:53 PM, Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> > I have no intention to change current include/require syntax, except
> adding
> > 2nd parameter.
>
> This is a bit misleading since include is not a function, so there's no
> 1st parameter. Instead, it's a syntax construct. Of course, syntax
> construct's grammar can be changed, though I'm not sure if that wouldn't
> lead to unexpected effects in the parser. But maybe not.
>

Although it is variable length parameters, we already have "echo".
I'll check it see if Zend has problem with 2nd parameter when I write patch.


>
> Bigger issue is that if somebody is knowledgeable enough in security to
> go and change their code to use this option, why they aren't
> knowledgeable enough to fix their includes so this option is not required?


The root cause of the issue here is preciseness of the setting.
I think you agree that current "allow_url_include=Off" with INI_SYSTEM is
not precise at all.

We need to consider local and remote wrapper separately.
We may better to consider removing all remote wrapper support from
include/require.
It's rarely used and user can execute remote script easily with PHP. e.g.
eval(readfile('http://host/script')).

That said, one other possible solution is disallow most local wrappers for
include/require
and restrict script file extension. AFAIK, phar and user wrappers are the
exception
for "allow_url_include=Off", but there may be others. If we remove most
local wrapper
support(php://input, user wrappers, etc) from include/require,  we don't
need 2nd parameter. i.e.

include('phar://phar_file.phar/script_in_phar.php'); // Requires .phar
extension to load PHP script from phar.

This requires users to change filename. Therefore, I propose to check
wrapper type
by 2nd parameter, but filename extension check works also. I don't mind to
use
this option.

There may be users that uses local wrapper to encrypt script files, etc. We
need
research if disabling most wrappers for include/require have problem or
not.

If there are other ideas, I appreciate it.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--001a1143254ecffcf705102e8935--