Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-23 Thread Yasuo Ohgaki
Hi Jan,

On Tue, Feb 24, 2015 at 12:51 AM, Jan Ehrhardt  wrote:

> Yasuo Ohgaki in php.internals (Mon, 23 Feb 2015 18:53:10 +0900):
> >On Mon, Feb 23, 2015 at 6:52 PM, Yasuo Ohgaki  wrote:
> >
> >> ini_set('.php .phar .module .etc');
> >
> >ini_set('zend.script_extensions', '.php .phar .module .etc');
> >
> >to be correct.
>
> Quote from a Drupal 7 .htaccess:
>
> # Protect files and directories from prying eyes.
> 
> "\.(engine|inc|info|install|make|module|profile|test|po|sh|.*sql|theme|tpl(\.php)?|xtmpl)$|^(\..*|Entries.*|Repository|Root|Tag|Template)$">
>   Order allow,deny
> 
>
> A lot of these, even .test, are used as include scripts. How would the
> corresponding .htaccess entry for zend.script_extensions be formulated?
>

My patch allows up to 32 extensions or disable the protection, so all of
these
can be used as PHP scripts by

ini_set('zend.script_extensions', ''); // The same as now. No mitigations.

I don't recommend disabling the protection nor adding too many extensions,
but it's up to developers.

By the way, the configuration is not for PHP script, but file access
control from
external clients. I suppose Drupal uses much less filename extensions for
PHP
scripts. Otherwise, it will be too weak against script inclusion (or direct
PHP
script access via uploaded files).

I checked Drupal8 files briefly. I see a few .module/.inc (there may be
others)
They may either rename to .module.php/.inc.php or

ini_set('zend.script_extensions', '.php .module .inc .phar');

I suggest developers to use standard extension for PHP scripts, use only a
few
extensions as PHP scripts if it's ever needed. Leaving
zend.script_extensions by
default and using '.php' as PHP script would be the best practice. I think
most
apps/scripts follow this already.

Regards,

P.S. My patch does not protect "direct access". i.e. It allows to execute
PHP
scripts specified by web server configurations when PHP is a module of web
server. e.g.


SetHandler application/x-http-php


allows ".png" files to be executed as PHP script regardless of
"zend.script_extensions".
PHP script files is opened by Web server and PHP will not check file
extension for
this case. The patch checks only when script file is loaded by PHP/Zend.

--
Yasuo Ohgaki
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-23 Thread Jan Ehrhardt
Yasuo Ohgaki in php.internals (Mon, 23 Feb 2015 18:53:10 +0900):
>On Mon, Feb 23, 2015 at 6:52 PM, Yasuo Ohgaki  wrote:
>
>> ini_set('.php .phar .module .etc');
>
>ini_set('zend.script_extensions', '.php .phar .module .etc');
>
>to be correct.

Quote from a Drupal 7 .htaccess:

# Protect files and directories from prying eyes.

  Order allow,deny


A lot of these, even .test, are used as include scripts. How would the
corresponding .htaccess entry for zend.script_extensions be formulated?

Jan

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-23 Thread Yasuo Ohgaki
On Mon, Feb 23, 2015 at 6:52 PM, Yasuo Ohgaki  wrote:

> ini_set('.php .phar .module .etc');


ini_set('zend.script_extensions', '.php .phar .module .etc');

to be correct.

--
Yasuo Ohgaki
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-23 Thread Yasuo Ohgaki
Hi Jan,

On Mon, Feb 23, 2015 at 6:32 PM, Jan Ehrhardt  wrote:

> Stanislav Malyshev in php.internals (Sun, 22 Feb 2015 14:00:02 -0800):
> >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.
>
> Exactly my point with the Drupal example. Drupal will break without
> changes.
>

They can adopt PHP7 by one liner.

ini_set('.php .phar .module .etc');

Since ini_set() does not raise any errors, it works all PHP versions.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-23 Thread Jan Ehrhardt
Stanislav Malyshev in php.internals (Sun, 22 Feb 2015 14:00:02 -0800):
>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.

Exactly my point with the Drupal example. Drupal will break without
changes.

Jan

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-22 Thread Yasuo Ohgaki
Hi Stas,

On Mon, Feb 23, 2015 at 7:00 AM, Stanislav Malyshev 
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
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-22 Thread Stanislav Malyshev
Hi!

> 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.

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.

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.

Also, the claim that move_upload_file() is obsolete is not based on
anything as far as I can see. Why is it "obsolete"?

-- 
Stas Malyshev
smalys...@gmail.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-21 Thread Yasuo Ohgaki
Hi Dan,

On Sun, Feb 22, 2015 at 12:40 AM, Dan Ackroyd 
wrote:

> From the RFC:
> > Patches and Tests
> > Will be prepared before vote.
>
> The implementation details may determine how some people vote. Is the
> patch still coming before the voting is opened?
>

Yes. The patch will be simple one.
I'll write it in a week.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-21 Thread Dan Ackroyd
>From the RFC:
> Patches and Tests
> Will be prepared before vote.

The implementation details may determine how some people vote. Is the
patch still coming before the voting is opened?

cheers
Dan

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-21 Thread Yasuo Ohgaki
Hi Padraic,

On Sat, Feb 21, 2015 at 5:18 PM, Pádraic Brady 
wrote:

> Does this have any impact on allow_url_include or has that setting
> been retained?
>
> Yes, folk do indeed try to do this, for example hitting up Google:
>
> http://www.quora.com/Why-do-include-and-require_once-not-work-with-remote-files
>

allow_url_include=Off is kept.

Attacker can easily place *.php files on remote servers.
I guess PHP also allows php://input without it, doesn't it?
php://input allows script execution via post.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net


Re: [PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-21 Thread Pádraic Brady
Does this have any impact on allow_url_include or has that setting
been retained?

Yes, folk do indeed try to do this, for example hitting up Google:
http://www.quora.com/Why-do-include-and-require_once-not-work-with-remote-files

Paddy

On 21 February 2015 at 01:06, Yasuo Ohgaki  wrote:
> Hi all,
>
> 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 hope everyone will like this proposal.
> Thank you all who have participated to discussions.
>
> Those who are not involved, this is the time to check this RFC.
>
> Thank you.
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net



-- 

--
Pádraic Brady

http://blog.astrumfutura.com
http://www.survivethedeepend.com
Zend Framework Community Review Team
Zend Framework PHP-FIG Representative

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] [RFC] [FINAL DISCUSSION] Script only include/require

2015-02-20 Thread Yasuo Ohgaki
Hi all,

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 hope everyone will like this proposal.
Thank you all who have participated to discussions.

Those who are not involved, this is the time to check this RFC.

Thank you.

--
Yasuo Ohgaki
yohg...@ohgaki.net