Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83881
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.49 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <54EEDE8E.6070201@gmail.com>
References: <CAGa2bXbt9FAdRtW1GxDyT+G-4bWBACTso_DOizcSHmmgmSVtmA@mail.gmail.com>
 <CAGa2bXb5QEKOggRbUnmVHuib=P_tJ9_W+QyMa_SCpc7RmSCgSA@mail.gmail.com>
 <54EE50CF.9090508@gmail.com> <CALwr1GmoAxfbKvJYVLeL3wPXEKKbu1jKGkTFcG0KQ_Ogx172sA@mail.gmail.com>
 <54EE5A39.9040401@gmail.com> <CAGa2bXYWQCZA8_-EpQcb_Af7DT-_FV7HdfAHZrVt4V7i7vEURg@mail.gmail.com>
 <54EEDE8E.6070201@gmail.com>
Date: Thu, 26 Feb 2015 19:01:44 +0900
Message-ID: <CAGa2bXamvrxJR=KBoygrwM=3RrHdB6cpuTJxcwVaY1jrUQ-4iQ@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11392386496b5f050ffad981
Subject: Re: [PHP-DEV] Re: [RFC][VOTE] Introduce script only include/require
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a11392386496b5f050ffad981
Content-Type: text/plain; charset=UTF-8

Hi Stas,

I'm on the side wishes PHP being more secure.
I think you are on the same side. Let's be productive for the objective :)

On Thu, Feb 26, 2015 at 5:51 PM, Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> > This can be prevented by restricting phar archive name or forbid all
> > URI name at all. The latter is better choice.
>
> If by "all uri" you mean all streams, that would be very high burden,
> which may break many applications using streams, including phar handling.
>

Phar has 2 issues.

1. It uses URI form for script, but allow_url_include is INI_SYSTEM.
2. Phar allows any filename extension including none.

Resolution for these requires BC. We may choose both or one of them.
If there is better idea, we may choose it also.


> > There is design problem obviously. The reason why phar:// is allowed is
> > that "allow_url_include" is not INI_ALL.
>
> The reason why phar is allowed is because phar is not a remote stream,
> so access to phar is the same as access to local file system, which is
> necessary for include anyway. The contents of phar is in the same domain
> as contents of local filesystem, that's why no distinction is made.
>

Local system files are relatively safe, I agree. However, we have many
users intruded by script inclusion attacks. We have to do something
effective.
I think you agree with this basic idea.

Although, your attack example via Phar file includes most forbidden char
"/" in
a filename, I think we are better to have mitigation/protection for phar
files.

The protection proposed here will be effective for require('phar://.. ')
for the
reason that it has forbidden char, but it's not safe enough.

We have to check php://input, zip://, etc more carefully to see if these
can do
bad thing. I'll do that later.


>
> > Script cannot change the setting, therefore special phar:// is needed to
> > be allowed unconditionally. I would say INI_SYSTEM for allow_url_include
> > is false sense of security. It introduced security issue like you've
> > mentioned.
>
> It doesn't introduce any issues, unless you declare there should be
> distinction between files on the filesystem, which is needed to make
> your approach useful. But I do not think it's a good security - trying
> to allow including user-controlled files securely is IMO a futile task.
> The problem should be fixed at the root - by not having such includes
> that allow arbitrary filenames injected by user.


I'm not checking stream wrappers yet, but it seems it's impossible for
users to
detect bad files if "local" stream wrappers are allowed always. Black
listing
almost never work well in this case.


In mean time, why don't we have proposed protection now? Most users
checks ".." and dir separators now. Our subject is "arbitrarily script
execution"
and it can be mitigated by this proposal.

We may add more effective protections/mitigations after research is done.
I volunteer for the task. It will be for PHP 7.1 or 7.2 unless there is
really fatal
issue.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--001a11392386496b5f050ffad981--