Re: [PHP-DEV] Re: [RFC] Script only include/require
Hi! > Will it add a significant level of protection? No. > > Does it add protection? Yes. > > Each time we add some incremental security hardening, we make it a bit > harder to create vulnerabilities. In this case, if there were code In this case, it seems not to be much harder than changing an URL a bit or uploading a file under different extension. OTOH, it creates a false sense of security - oh, I'm using the secure settings, now I can forget about caring for LFI! - and also has huge BC break potential. For me, it looks like magic quotes comeback. > injection issue, the attacker must a) include a local file (not always > useful) or b) upload some other apparently innocent file capable of > being included (extremely useful). As such, this patch would lock out > an obvious path by restricting the files that can be included to a > more limited subset. Unless you disable phar, you can still include pretty much anything by just using phar includes, as far as I can see. I'm pretty sure there are also other stream tricks possible (data://? zip://?) > Enough incremental improvements add up to a significant improvement. If that were always true, safe mode and magic quotes would still be here with us. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Script only include/require
Hi Dmitry, On 24 February 2015 at 07:00, Dmitry Stogov wrote: > I'm not a security expert, but I think that adding check for script > extension won't add significant level of protection. Will it add a significant level of protection? No. Does it add protection? Yes. Each time we add some incremental security hardening, we make it a bit harder to create vulnerabilities. In this case, if there were code injection issue, the attacker must a) include a local file (not always useful) or b) upload some other apparently innocent file capable of being included (extremely useful). As such, this patch would lock out an obvious path by restricting the files that can be included to a more limited subset. Enough incremental improvements add up to a significant improvement. Paddy -- Pádraic Brady http://blog.astrumfutura.com http://www.survivethedeepend.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [RFC] Script only include/require
Hi all, On Tue, Feb 24, 2015 at 7:20 PM, Yasuo Ohgaki wrote: > On Tue, Feb 24, 2015 at 4:00 PM, Dmitry Stogov wrote: > >> Use E_ERROR. >> >> >>> >>> >>> https://github.com/php/php-src/pull//files#diff-93ad74868f98ff7232ebea7c8b7fR624 >>> >>> Does engine exception catches error from zend_error_noreturn()? >>> >> >> no. it'll be changed into zend_error(). >> > > Thank you for the comment. > > I'm not a security expert, but I think that adding check for script >> extension won't add significant level of protection. >> > > I agree. For developers who have more than average skills, this RFC > would not be helpful. File inclusions by readfile()/etc are fatal as well > also. Users must be careful anyway. > > My objective is to reduce risk of server takeover by script inclusions > as low as other languages and being nice to new developers. I've audited > number of web applications written by various languages, there aren't much > difference in programmers' skills. My samples are too few and do not > represent actual figures, but we'll have less vulnerable PHP apps by this. > IMHO. > I would like to show one common example that is unique to PHP. https://www.google.co.jp/search?q=Exif+Webshell+Backdoor This RFC prevents this type of attack effectively. All users has to do is "checking file extension is image". Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Re: [RFC] Script only include/require
Hi Dmitry, On Tue, Feb 24, 2015 at 4:00 PM, Dmitry Stogov wrote: > Use E_ERROR. > > >> >> >> https://github.com/php/php-src/pull//files#diff-93ad74868f98ff7232ebea7c8b7fR624 >> >> Does engine exception catches error from zend_error_noreturn()? >> > > no. it'll be changed into zend_error(). > Thank you for the comment. I'm not a security expert, but I think that adding check for script > extension won't add significant level of protection. > I agree. For developers who have more than average skills, this RFC would not be helpful. File inclusions by readfile()/etc are fatal as well also. Users must be careful anyway. My objective is to reduce risk of server takeover by script inclusions as low as other languages and being nice to new developers. I've audited number of web applications written by various languages, there aren't much difference in programmers' skills. My samples are too few and do not represent actual figures, but we'll have less vulnerable PHP apps by this. IMHO. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Re: [RFC] Script only include/require
On Mon, Feb 23, 2015 at 6:55 AM, Yasuo Ohgaki wrote: > Hi Dmitry and Nikita, > > On Mon, Feb 23, 2015 at 6:23 AM, Yasuo Ohgaki wrote: > >> I wrote patch and made adjustment in the RFC >> https://wiki.php.net/rfc/script_only_include >> https://github.com/php/php-src/pull/ >> Where to check filename extension is subject to be changed. >> At first, I thought implementing this as PHP code is good, but >> I've changed my mind. It seems better to be done in Zend code. >> Opinions are appreciated. >> >> This RFC aims to make PHP as secure as other languages >> with respect to "script inclusion" attacks. >> Note: File inclusion is not a scope of this RFC. >> >> INI Changes: >> - "php_script" -> "zend.script_extensions" >> - "Allow all files": "*" -> NULL or "" >> >> Open Issues: >> - Error type - Is it OK to raise E_ERROR/E_RECOVERABLE_ERROR in >>zend_language_scanner.c? >> - Vote type - 50%+1 or 2/3 >> >> If there is anyone who would like to vote "no" for this RFC, >> I would like to know the reason and try to address/resolve issue you have. >> >> Thank you. >> > > We don't have care much about which error is raised from Zend engine, > since there > will be engine exception. > > My questions are, is it ok to raise E_ERROR or E_RECOVERABLE_ERROR from > zend_language_scanner.c? > Use E_ERROR. > > > https://github.com/php/php-src/pull//files#diff-93ad74868f98ff7232ebea7c8b7fR624 > > Does engine exception catches error from zend_error_noreturn()? > no. it'll be changed into zend_error(). I'm not a security expert, but I think that adding check for script extension won't add significant level of protection. Thanks. Dmitry. > > Thank you. > > Regards, > > -- > Yasuo Ohgaki > yohg...@ohgaki.net > >
Re: [PHP-DEV] Re: [RFC] Script only include/require
Hi Stas, On Mon, Feb 23, 2015 at 5:02 PM, Stanislav Malyshev wrote: > > I noticed very strange behavior under ZTS build with this patch. > > It turned out that compiler_globals is not accessible under ZTS build > > according to gdb. > > > > Is this intended? If so, where should I put script_extensions char array? > > That doesn't look right. If compiler_globals weren't accessible nothing > would work in ZTS. It may be some bug or missing setting in gdb. Thank you. Now I see compiler globals under ZTS in gdb and it looks OK. The execution steps do not make sense at all in gdb, but simply commenting out new code for move_uploaded_file() makes move_uploaded_file() work again. It seems I have to read machine code to see what's wrong :( Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Re: [RFC] Script only include/require
Hi! > I noticed very strange behavior under ZTS build with this patch. > It turned out that compiler_globals is not accessible under ZTS build > according to gdb. > > Is this intended? If so, where should I put script_extensions char array? That doesn't look right. If compiler_globals weren't accessible nothing would work in ZTS. It may be some bug or missing setting in gdb. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [RFC] Script only include/require
Hi all, Zend engine experts especially, On Mon, Feb 23, 2015 at 6:23 AM, Yasuo Ohgaki wrote: > I wrote patch and made adjustment in the RFC > https://wiki.php.net/rfc/script_only_include > https://github.com/php/php-src/pull/ > Where to check filename extension is subject to be changed. > At first, I thought implementing this as PHP code is good, but > I've changed my mind. It seems better to be done in Zend code. > Opinions are appreciated. > I noticed very strange behavior under ZTS build with this patch. It turned out that compiler_globals is not accessible under ZTS build according to gdb. Is this intended? If so, where should I put script_extensions char array? Thank you. -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Re: [RFC] Script only include/require
Hi Dmitry and Nikita, On Mon, Feb 23, 2015 at 6:23 AM, Yasuo Ohgaki wrote: > I wrote patch and made adjustment in the RFC > https://wiki.php.net/rfc/script_only_include > https://github.com/php/php-src/pull/ > Where to check filename extension is subject to be changed. > At first, I thought implementing this as PHP code is good, but > I've changed my mind. It seems better to be done in Zend code. > Opinions are appreciated. > > This RFC aims to make PHP as secure as other languages > with respect to "script inclusion" attacks. > Note: File inclusion is not a scope of this RFC. > > INI Changes: > - "php_script" -> "zend.script_extensions" > - "Allow all files": "*" -> NULL or "" > > Open Issues: > - Error type - Is it OK to raise E_ERROR/E_RECOVERABLE_ERROR in >zend_language_scanner.c? > - Vote type - 50%+1 or 2/3 > > If there is anyone who would like to vote "no" for this RFC, > I would like to know the reason and try to address/resolve issue you have. > > Thank you. > We don't have care much about which error is raised from Zend engine, since there will be engine exception. My questions are, is it ok to raise E_ERROR or E_RECOVERABLE_ERROR from zend_language_scanner.c? https://github.com/php/php-src/pull//files#diff-93ad74868f98ff7232ebea7c8b7fR624 Does engine exception catches error from zend_error_noreturn()? Thank you. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
