# [RFC] Script only include/require - Thread: https://externals.io/message/83488 - Original archive: https://news-web.php.net/php.internals/83488 - Started by Yasuo Ohgaki on 2015-02-22T21:23:35+00:00 - Messages: 44 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/83488.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #83488 — Yasuo Ohgaki — 2015-02-22T21:23:35+00:00 URL: https://externals.io/message/83488#83488 Hi all, I wrote patch and made adjustment in the RFC https://wiki.php.net/rfc/script_only_include https://github.com/php/php-src/pull/1111 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. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #83538 — Yasuo Ohgaki — 2015-02-23T03:55:43+00:00 URL: https://externals.io/message/83488#83538 · In reply to: #83488 Hi Dmitry and Nikita, > I wrote patch and made adjustment in the RFC > https://wiki.php.net/rfc/script_only_include > https://github.com/php/php-src/pull/1111 > 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/1111/files#diff-93ad74868f98ff7232ebea00007c8b7fR624 Does engine exception catches error from zend_error_noreturn()? Thank you. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #83547 — Yasuo Ohgaki — 2015-02-23T06:53:31+00:00 URL: https://externals.io/message/83488#83547 · In reply to: #83488 Hi all, Zend engine experts especially, > I wrote patch and made adjustment in the RFC > https://wiki.php.net/rfc/script_only_include > https://github.com/php/php-src/pull/1111 > 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 yohgaki@ohgaki.net --- ## #83548 — Stanislav Malyshev — 2015-02-23T08:02:59+00:00 URL: https://externals.io/message/83488#83548 · In reply to: #83547 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 smalyshev@gmail.com --- ## #83552 — Yasuo Ohgaki — 2015-02-23T08:54:30+00:00 URL: https://externals.io/message/83488#83552 · In reply to: #83548 Hi Stas, On Mon, Feb 23, 2015 at 5:02 PM, Stanislav Malyshev wrote: > > […] > > 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 yohgaki@ohgaki.net --- ## #83633 — Dmitry Stogov — 2015-02-24T07:00:02+00:00 URL: https://externals.io/message/83488#83633 · In reply to: #83538 > Hi Dmitry and Nikita, > > > > > […] > > 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/1111/files#diff-93ad74868f98ff7232ebea00007c8b7fR624 > > 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 > yohgaki@ohgaki.net --- ## #83648 — Yasuo Ohgaki — 2015-02-24T10:20:27+00:00 URL: https://externals.io/message/83488#83648 · In reply to: #83633 Hi Dmitry, > Use E_ERROR. > > > > […] > > 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 yohgaki@ohgaki.net --- ## #83649 — Yasuo Ohgaki — 2015-02-24T10:39:42+00:00 URL: https://externals.io/message/83488#83649 · In reply to: #83648 Hi all, > > > > […] > > Thank you for the comment. > > I'm not a security expert, but I think that adding check for script > > […] > > 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 yohgaki@ohgaki.net --- ## #83656 — padraic.brady@gmail.com — 2015-02-24T11:56:38+00:00 URL: https://externals.io/message/83488#83656 · In reply to: #83633 Hi Dmitry, > 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 --- ## #83692 — Stanislav Malyshev — 2015-02-24T19:45:39+00:00 URL: https://externals.io/message/83488#83692 · In reply to: #83656 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 smalyshev@gmail.com --- ## #83696 — padraic.brady@gmail.com — 2015-02-24T20:33:17+00:00 URL: https://externals.io/message/83488#83696 · In reply to: #83692 Hi On Tuesday, February 24, 2015, Stanislav Malyshev wrote: > Hi! > > > […] > > 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. They'd need to upload with a matching file type. Instead of any file types. Fewer possible types is by definition less than all types. This is not even remotely magic quotes. No input is altered. > > > […] > > 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://?) None of this detracts from limiting file includes. Other potential weaknesses could be addressed separately if you agree there's more than one addressed not addressed here. One might say...incrementally. Also, we are obviously talking about PHP includes with this RFC... > > […] > > If that were always true, safe mode and magic quotes would still be here > with us. > You keep mentioning magic quotes. That was never an improvement. It was removed from PHP. Please stop trying to associate two unrelated things to establish bad practice by word proximity. The sentence is obviously true. Paddy -- -- Pádraic Brady http://blog.astrumfutura.com http://www.survivethedeepend.com Zend Framework Community Review Team Zend Framework PHP-FIG Representative --- ## #83698 — Yasuo Ohgaki — 2015-02-24T21:15:42+00:00 URL: https://externals.io/message/83488#83698 · In reply to: #83696 Hi Stas, On Wed, Feb 25, 2015 at 5:33 AM, Pádraic Brady wrote: > On Tuesday, February 24, 2015, Stanislav Malyshev > wrote: > > > […] > > > They'd need to upload with a matching file type. Instead of any file > types. Fewer possible types is by definition less than all types. > > This is not even remotely magic quotes. No input is altered. > I would like to add a note for this. Anti Virus products are detecting this type of files as "PHP malware". No other languages have such malware. According to recent F-Secure blog post, this type of "PHP malware" files are not decreasing but increasing. Other than this type of "PHP malware", "PHP WebShell" is detected as PHP malware by anti virus products. The reason why these has to detected as "PHP malware" is that there are PHP programs vulnerable to script inclusion attacks. Leaving this as it is now would make people think "PHP is insecure than other languages", "Wow, we have many PHP malware. We may be better not to use PHP anymore". If "PHP malware" is found in a server, developers are force to check their code. Or they have to ask costly code check to people like me, even when PHP programs is safe. If this RFC is accepted, developers can prove their PHP programs are safe without code check. This RFC benefits may not be obvious for people on this list, but this RFC eliminates certain type of "PHP malware". PHP's script inclusion is a toy for security researcher and attackers for a long time. Let's take away the toy from them. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #83704 — Stanislav Malyshev — 2015-02-24T22:07:01+00:00 URL: https://externals.io/message/83488#83704 · In reply to: #83696 Hi! > They'd need to upload with a matching file type. Instead of any file Not sure what you mean by that. phar can read tars, etc. AFAIK, can't it? Also, phar archive has no requirement of being named something.phar, afaik can be also named cuteponies.gif. E.g., I just did this: 1. Created file chump.php: This is not even remotely magic quotes. No input is altered. Don't be so literal. It's not about altering input, it's about the fact that it breaks stuff and not adds much to security. > None of this detracts from limiting file includes. Other potential Not sure what you mean. If you can pull off file include - which is a precondition of this feature being useful - then you can pull off phar include. > weaknesses could be addressed separately if you agree there's more than > one addressed not addressed here. One might say...incrementally. The problem is there's no increment there. It's like having a password hardcoded to "password". You can say "oh, it's incremental security, at least we have a password!" but it is not incrementing the actual security. > You keep mentioning magic quotes. That was never an improvement. It was > removed from PHP. Please stop trying to associate two unrelated things Yes, it was removed from PHP - exactly because it did not produce the attempted improvement in security. This feature is of the same kind - it tries to produce increase in security but fails. Thinking of it as a security feature would produce nothing but an endless stream of CVEs with PHP name attached to it. Not a good idea. -- Stas Malyshev smalyshev@gmail.com --- ## #83706 — Yasuo Ohgaki — 2015-02-24T22:24:31+00:00 URL: https://externals.io/message/83488#83706 · In reply to: #83704 Hi Stas, On Wed, Feb 25, 2015 at 7:07 AM, Stanislav Malyshev wrote: > > […] > > Not sure what you mean by that. phar can read tars, etc. AFAIK, can't > it? Also, phar archive has no requirement of being named something.phar, > afaik can be also named cuteponies.gif. E.g., I just did this: > > 1. Created file chump.php: > > > include $argv[1]; > > This is an idealized vulnerable script. > > 2. Created file pwnd.php > > echo "pwnd!"; > > This is an idealized exploit. > > 3. Put it into an archive: > tar cvzf cuteponies.gif pwnd.php > > 4. Run this: > > php -dallow_url_include=0 chump.php phar://cuteponies.gif/pwnd.php > > The output is: > > pwnd! > > I'm not sure how this measure would protect from such scenario. Am I > missing something here? I think he means matching file "extension". File extension should represent file type, though. The new RFC check filename extensions. It allows only ".php", ".phar" as PHP script and `move_uploaded_file()` restricts moving PHP scripts by default. (Old idea was to detect PHP script by contents. New RFC is to restrict PHP script file extension.) Since "pwnd.php" has ".php" extension, `move_uploaded_file()` refuses to move it to upload dir by default. As long as user uses default and `move_uploaded_file()`, they are free from script upload attacks including embedded script. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #83707 — Stanislav Malyshev — 2015-02-24T22:26:48+00:00 URL: https://externals.io/message/83488#83707 · In reply to: #83698 Hi! > I would like to add a note for this. > Anti Virus products are detecting this type of files as "PHP malware". It looks like you are trying to convince me that PHP malware exists. I would like to save you time by notifying you I am aware of this. My disagreement is not denying PHP malware exists, it is denying that your proposed change does anything to improve situations with code vulnerable to externally controlled includes. There may be a way to mitigate this problem, but I don't see how requiring that .php would be at the end of the filename would be it. > No other languages have such malware. Are you seriously claiming there is no malware written in languages besides PHP? It can not be, I must be misunderstanding what you mean here. > The reason why these has to detected as "PHP malware" is that there are > PHP programs vulnerable to script inclusion attacks. No, it's not the reason, at least not the main one. The reason is that: a. PHP is an easy language to write code in and is widely deployed b. Writing a remote control kit in PHP is easier than in C, etc. and there's more guarantee it would work on any random PHP host c. There are lots of vulnerable web hosts that have remote execution vulnerabilities and can be exploited > Leaving this as it is now would make people think "PHP is insecure than > other languages", "Wow, we have many PHP malware. We may be better > not to use PHP anymore". People that think that are illogical - the fact that somebody chose to write a remote control toolkit in PHP due to PHP'd high footprint on the web has absolutely nothing to do with PHP being less secure. It's like saying Ford cars are insecure because somebody robbed a bank and then drove away in a Ford car. We should pay absolutely zero attention to the opinion of people that are so confused, and instead educate them about the real situation. Of course, if people run no PHP server at all, PHP-driven remote control kits would not be useful. But if the server is vulnerable, there are many other backdoor kits. > If "PHP malware" is found in a server, developers are force to check > their code. Or they have to ask costly code check to people like me, > even when PHP programs is safe. If this RFC is accepted, developers > can prove their PHP programs are safe without code check. I do not see how you change leads to anything like that. > This RFC benefits may not be obvious for people on this list, but this > RFC eliminates certain type of "PHP malware". PHP's script inclusion I can't think of any type of PHP malware that would be eliminated. At most, the malware injection protocols have to be slightly modified to work around initial hurdle of not being able to pass files with extension .php through move_upload_file(). With RCE vulnerability its trivial, with RFI one based on uploads it is a little harder, but only insignificantly - if I am not mistaken, in the last email I provided a workaround and it took me less than 5 minutes to come up with it, without being professional exploit writer. > is a toy for security researcher and attackers for a long time. > Let's take away the toy from them. It may be worth to take away the toy, but this change just moves the toy couple of centimeters aside. Given the BC break potential, I don't see much point. -- Stas Malyshev smalyshev@gmail.com --- ## #83708 — Stanislav Malyshev — 2015-02-24T22:31:03+00:00 URL: https://externals.io/message/83488#83708 · In reply to: #83706 Hi! > I think he means matching file "extension". File extension should > represent file type, though. You can not rely on that. I can name files anything regardless of what's in the file. > Since "pwnd.php" has ".php" extension, `move_uploaded_file()` refuses to > move it > to upload dir by default. There's no pwnd.php. The file that I upload is "cuteponies.gif". Please look at the sequence again carefully. -- Stas Malyshev smalyshev@gmail.com --- ## #83710 — Yasuo Ohgaki — 2015-02-24T22:53:33+00:00 URL: https://externals.io/message/83488#83710 · In reply to: #83708 Hi Stas, On Wed, Feb 25, 2015 at 7:31 AM, Stanislav Malyshev wrote: > > […] > > You can not rely on that. I can name files anything regardless of what's > in the file. > > > […] > > There's no pwnd.php. The file that I upload is "cuteponies.gif". Please > look at the sequence again carefully. require('cuteponies.gif) wouldn't work with this RFC. move_uploaded_files() prohibits uploading PHP script. I noticed that I should forbid destination file extension also by this discussion. I'll add it soon. Thank you. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #83711 — Yasuo Ohgaki — 2015-02-24T22:54:01+00:00 URL: https://externals.io/message/83488#83711 · In reply to: #83707 Hi Stas, On Wed, Feb 25, 2015 at 7:26 AM, Stanislav Malyshev wrote: > > […] > > It looks like you are trying to convince me that PHP malware exists. I > would like to save you time by notifying you I am aware of this. My > disagreement is not denying PHP malware exists, it is denying that your > proposed change does anything to improve situations with code vulnerable > to externally controlled includes. There may be a way to mitigate this > problem, but I don't see how requiring that .php would be at the end of > the filename would be it. > I'm presenting the fact that "PHP script embedded malware" exists in the wild and malware vendors detect them as "PHP malware". > > > […] > > Are you seriously claiming there is no malware written in languages > besides PHP? It can not be, I must be misunderstanding what you mean here. > Malwares are written by many languages. It's the fact. As far as I know, PHP is the only language that has this type of malware. (Script embedded images) PHP is the only one malware vendors claims it as "PHP malware". This is the fact. > The reason why these has to detected as "PHP malware" is that there are > > […] > > No, it's not the reason, at least not the main one. The reason is that: > a. PHP is an easy language to write code in and is widely deployed > b. Writing a remote control kit in PHP is easier than in C, etc. and > there's more guarantee it would work on any random PHP host > c. There are lots of vulnerable web hosts that have remote execution > vulnerabilities and can be exploited > The reason is other languages are almost safe by default against script inclusion attacks. This RFC makes PHP safe by default just like other languages + `move_uploaded_file()` protection. > > > […] > > People that think that are illogical - the fact that somebody chose to > write a remote control toolkit in PHP due to PHP'd high footprint on the > web has absolutely nothing to do with PHP being less secure. It's like > saying Ford cars are insecure because somebody robbed a bank and then > drove away in a Ford car. We should pay absolutely zero attention to the > opinion of people that are so confused, and instead educate them about > the real situation. > > Of course, if people run no PHP server at all, PHP-driven remote control > kits would not be useful. But if the server is vulnerable, there are > many other backdoor kits. > People do not have to be exparts of developing softwares. Managers will choose illogical choice. > If "PHP malware" is found in a server, developers are force to check > > […] > > I do not see how you change leads to anything like that. > 1. Anti-virus detects "PHP malware" 2. Managers surprises possible attack (Server takeover) 3. Developers are forced to check their code, since current PHP has no effective script inclusion attack prevention With this RFC, developers can explain this type of attacks cannot be done by PHP's feature. i.e. Exploit servers via script embedded images, etc cannot be done. > This RFC benefits may not be obvious for people on this list, but this > > […] > > I can't think of any type of PHP malware that would be eliminated. At > most, the malware injection protocols have to be slightly modified to > work around initial hurdle of not being able to pass files with > extension .php through move_upload_file(). With RCE vulnerability its > trivial, with RFI one based on uploads it is a little harder, but only > insignificantly - if I am not mistaken, in the last email I provided a > workaround and it took me less than 5 minutes to come up with it, > without being professional exploit writer. > Embedded PHP script uploads are prohibited by this RFC by default. > is a toy for security researcher and attackers for a long time. > > […] > > It may be worth to take away the toy, but this change just moves the toy > couple of centimeters aside. Given the BC break potential, I don't see > much point. PHP became as secure as other languages with respect to script inclusions. by default. The issue here is "PHP is not being as secure as other language with respect to script inclusion attacks". Statistics shows it. This is our issue. Users may shoot their own foot, this is not our issue. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #83713 — Yasuo Ohgaki — 2015-02-24T23:00:21+00:00 URL: https://externals.io/message/83488#83713 · In reply to: #83710 Hi Stas, > require('cuteponies.gif) wouldn't work with this RFC. > move_uploaded_files() prohibits uploading PHP script. > I noticed that I should forbid destination file extension also by this > discussion. > I'll add it soon. Thank you. > Oops, I did it right. I forbid destination path to be PHP script. No change is required to `move_uploaded_file()`. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #83714 — Stanislav Malyshev — 2015-02-24T23:06:01+00:00 URL: https://externals.io/message/83488#83714 · In reply to: #83710 Hi! > require('cuteponies.gif) wouldn't work with this RFC. > move_uploaded_files() prohibits uploading PHP script. You seem not to be reading the scenario. The include URL would be phar://cuteponies.gif/pwnd.php and the uploaded file would be cuteponies.gif. Your protection would not stop moving .gif file, and your filename check would pass phar://cuteponies.gif/pwnd.php since it ends in .php. -- Stas Malyshev smalyshev@gmail.com --- ## #83715 — Stanislav Malyshev — 2015-02-24T23:25:16+00:00 URL: https://externals.io/message/83488#83715 · In reply to: #83711 Hi! > As far as I know, PHP is the only language that has this type of malware. > (Script embedded images) PHP is the only one malware vendors claims > it as "PHP malware". This is the fact. Which type is that? Of course only malware in PHP can be presented as "PHP malware", but I don't understand why it is of any significance. > The reason is other languages are almost safe by default against script > inclusion attacks. Many languages just don't use patterns like PHP code does - I don't think I ever seen Python code doing imports based on variables - I'm not even sure it's possible in Python. PHP has more capabilities, but of course you need to use them in the right way. > This RFC makes PHP safe by default just like other languages + > `move_uploaded_file()` > protection. No, it does not - I've shown the example why. I'm sure there are more. > People do not have to be exparts of developing softwares. Managers will > choose illogical choice. We should not base our decision on the opinions of people we all understand are ignorant. > 1. Anti-virus detects "PHP malware" > 2. Managers surprises possible attack (Server takeover) > 3. Developers are forced to check their code, since current PHP has no > effective script inclusion attack prevention > > With this RFC, developers can explain this type of attacks cannot be done > by PHP's feature. i.e. Exploit servers via script embedded images, etc > cannot > be done. I don't think we need to introduce BC-breaking feature in PHP just because somebody has a manager that can't understand the basics of security. > Embedded PHP script uploads are prohibited by this RFC by default. Only certain very narrow cases of it. > > PHP became as secure as other languages with respect to script inclusions. You keep repeating that, but it's not the case, and PHP already is as secure as other languages - provided you do not use clearly broken code. Security of the language is a misnomer anyway - language can not be secure (unless it's a language that does nothing useful), only specific code can be. Code that allows user-controlled includes without adequate filtering is insecure, and pretending that we make it secure does not improve security, quite the contrary. > Users may shoot their own foot, this is not our issue. But that's exactly what is required for your change to be useful at all! -- Stas Malyshev smalyshev@gmail.com --- ## #83719 — padraic.brady@gmail.com — 2015-02-25T00:09:39+00:00 URL: https://externals.io/message/83488#83719 · In reply to: #83704 Hi, > Hi! > > > […] > > Not sure what you mean by that. phar can read tars, etc. AFAIK, can't > it? Also, phar archive has no requirement of being named something.phar, > afaik can be also named cuteponies.gif. E.g., I just did this: Your example omitted the image validation step which would have noticed your attempt to upload a phar immediately. Add that and try again. It's not very fair to create a scenario with a total lack of any security, and then ignore that your code's problem is that gaping hole and NOT the minor extension filter on the far end. The control under debate was already provided with a preventable example by Yasuo pointing out how certain crafted images for file inclusion, which would bypass certain image validation checks, would indeed be preventable by his RFC. Please stick to what the RFC actually claims to do. Paddy -- Pádraic Brady http://blog.astrumfutura.com http://www.survivethedeepend.com --- ## #83720 — padraic.brady@gmail.com — 2015-02-25T00:20:17+00:00 URL: https://externals.io/message/83488#83720 · In reply to: #83707 Hi, > > […] > > I can't think of any type of PHP malware that would be eliminated. At > most, the malware injection protocols have to be slightly modified to > work around initial hurdle of not being able to pass files with > extension .php through move_upload_file(). With RCE vulnerability its > trivial, with RFI one based on uploads it is a little harder, but only > insignificantly - if I am not mistaken, in the last email I provided a > workaround and it took me less than 5 minutes to come up with it, > without being professional exploit writer. You might want to carefully read Yasuo's sentence about "certain" types which is not the same as "all" types. You seem to be exaggerating the claimed benefit of the RFC and using those exaggerated claims (and their debunking) as evidence against the RFC. In this, you are seriously off topic. The RFC makes a very simple claim about limiting includes to specific file extensions. It does not validate the files - the implicit assumption is the files are pre-validated so it exists to mop up certain edge cases that may bypass validation. This is just basic defense in depth. Paddy -- Pádraic Brady http://blog.astrumfutura.com http://www.survivethedeepend.com --- ## #83721 — Stanislav Malyshev — 2015-02-25T00:23:39+00:00 URL: https://externals.io/message/83488#83721 · In reply to: #83719 Hi! > Your example omitted the image validation step which would have Ah, right, and if I name it .zip, it'd be zip validation, and if I name it .pdf it'd be pdf validation, and if I name it .lol that would be LOL validation. You'd have to manually validate every type in existence and somehow invent validation for unknown ones. And it's not like I can't also make it a valid GIF/PDF/whatever - that has been done already. So you support this "security measure" by saying you can maybe plug the hole using other measures. Maybe you can, maybe (more probably) not but that does not redeem the insecurity of this "security measure". > again. It's not very fair to create a scenario with a total lack of > any security, and then ignore that your code's problem is that gaping > hole and NOT the minor extension filter on the far end. But that is *exactly* what this RFC is doing! The gaping security hole is include $argv[1] and that's where it should be fixed, not introducing temporary patches that prevent 1% of the scenarios and can be overridden within minutes. I don't see how it is not fair since the *only* scenario where it is useful is when your code *already* has the gaping security hole, otherwise this RFC has no utility as your includes are controlled by you so they already are php files. > indeed be preventable by his RFC. Please stick to what the RFC > actually claims to do. It claims to protect from file inclusion, by only allowing for include to operate on strings which end in .php, and then banning such files (ending in .php) from being handled by `move_uploaded_file()`. As I demonstrated (and this is I suspect not the only option) this does not actually offer any protection from LFI/RCE, as the end of the string given to include and the file on disk do not have to be the same. In my eyes, mechanism with such big BC break potential that is overridden in so trivial manner has little value. That even not considering upload doesn't even have to use `move_uploaded_file()` either. -- Stas Malyshev smalyshev@gmail.com --- ## #83722 — Yasuo Ohgaki — 2015-02-25T00:33:44+00:00 URL: https://externals.io/message/83488#83722 · In reply to: #83715 Hi Stas, On Wed, Feb 25, 2015 at 8:25 AM, Stanislav Malyshev wrote: > > […] > > Which type is that? Of course only malware in PHP can be presented as > "PHP malware", but I don't understand why it is of any significance. > I showed Exif specific one as "an" example. Regardless of your view, people recognizes "PHP is weaker against script inclusion" because there is "malware works with PHP", but not other languages. > > > […] > > Many languages just don't use patterns like PHP code does - I don't > think I ever seen Python code doing imports based on variables - I'm not > even sure it's possible in Python. PHP has more capabilities, but of > course you need to use them in the right way. > To do a similar thing as PHP does, Python needs to read file and parse, then execute. It's a lot of work compare to "require('file');" Ruby is a little easier since it allows "full path" script loading. However, most developers omit file extension. In addition, Ruby does not allow embedded scripting unlike PHP. It's much harder to make "Script Inclusion vulnerability" in other languages. > > > […] > > No, it does not - I've shown the example why. I'm sure there are more. > With this RFC - require/include only includes ".php" ".phar" by default. - move_uplaoded_file() only move files other than ".php", ".phar" extension by default. This makes "script inclusion" attack much harder. In fact, this RFC makes almost impossible attackers to take advantage of PHP's embed everything nature. > > > […] > > We should not base our decision on the opinions of people we all > understand are ignorant. > > > […] > > I don't think we need to introduce BC-breaking feature in PHP just > because somebody has a manager that can't understand the basics of > security. > For users do not need these protections, they can simply add one liner. ini_set('zend.script_extensions', ''); This piece of code is executed for all PHP version without any errors. > > > […] > > Only certain very narrow cases of it. > Unlike previous RFC, this RFC patch does not care "contents" of file, but only cares file "extensions". However, it works well as described before. > > > […] > inclusions. > > You keep repeating that, but it's not the case, and PHP already is as > secure as other languages - provided you do not use clearly broken code. > Security of the language is a misnomer anyway - language can not be > secure (unless it's a language that does nothing useful), only specific > code can be. Code that allows user-controlled includes without adequate > filtering is insecure, and pretending that we make it secure does not > improve security, quite the contrary. > I think I explained enough in previous comments. > > > […] > > But that's exactly what is required for your change to be useful at all! Even with this RFC, users can shoot their own foot and PHP is much easier to do it wrong compare to other languages. However, if users use new PHP by default setting, PHP is as safe as other languages against script inclusion/upload attacks. Since we have `move_uploaded_file()` protection, PHP becomes safer than others. - require/include cannot load script other than ".php"/".phar". - move_uploaded_files() cannot move script files ".php"/".phar". Script inclusion is one of the most severe security breach. This RFC works well for it. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- Next page: https://externals.io/message/83488.md?page=2