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": "*" ->
NULLor ""
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
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": "*" ->
NULLor ""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
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": "*" ->
NULLor ""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_ERRORorE_RECOVERABLE_ERRORfrom
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
Hi Dmitry,
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().
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
Hi all,
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().
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 byreadfile()/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
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
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
Hi
On Tuesday, February 24, 2015, Stanislav Malyshev smalyshev@gmail.com
wrote:
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 codeIn 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.
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://?)
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...
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.
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
Hi Stas,
On Wed, Feb 25, 2015 at 5:33 AM, Pádraic Brady padraic.brady@gmail.com
wrote:
On Tuesday, February 24, 2015, Stanislav Malyshev smalyshev@gmail.com
wrote: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 codeIn 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.
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
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
Hi Stas,
On Wed, Feb 25, 2015 at 7:26 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:
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.
I'm presenting the fact that "PHP script embedded malware" exists
in the wild and malware vendors detect them as "PHP malware".
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.
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
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
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.
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.
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
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.
- Anti-virus detects "PHP malware"
- Managers surprises possible attack (Server takeover)
- 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
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.
Embedded PHP script uploads are prohibited by this RFC by default.
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.
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
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.
- Anti-virus detects "PHP malware"
- Managers surprises possible attack (Server takeover)
- Developers are forced to check their code, since current PHP has no
effective script inclusion attack preventionWith 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
Hi Stas,
On Wed, Feb 25, 2015 at 8:25 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:
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.
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.
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.
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.
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.
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.
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.
- Anti-virus detects "PHP malware"
- Managers surprises possible attack (Server takeover)
- Developers are forced to check their code, since current PHP has no
effective script inclusion attack preventionWith 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.
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.
Embedded PHP script uploads are prohibited by this RFC by default.
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.
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.
I think I explained enough in previous comments.
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!
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
Hi!
- require/include only includes ".php" ".phar" by default.
This is not true. As I repeatedly point out, your change only requires
that the string passed to include would end in .php, but string passed
to include and filename on filesystem are very different things, they do
not have to be the same at all!
- 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.
I am at loss how you can claim it is "impossible" after I just
demonstrated how it is possible.
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.
You sincerely do not understand why breaking all code that uses non-php
extensions is a problem and that editing existing production code's
entry points is by no way "simple"? That finding all extensions that may
be used for files in a complex software is very far from "simple"?
Unlike previous RFC, this RFC patch does not care "contents" of file, but
only cares file "extensions". However, it works well as described before.
It doesn't work well - in fact, it does not work at all, as I just
demonstrated, exactly because extension of the actual file and what
you're checking are different things.
Even with this RFC, users can shoot their own foot and PHP is much easier
to do it wrong compare to other languages.
These platitudes do not change the simple fact that your RFC does not do
what it claims to do, security-wise.
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.
No, it does not become safer, because if you have code that allows
script inclusion injection (and if you do, it's your fault, not PHP's)
it is trivial to exploit it even with this RFC.
- require/include cannot load script other than ".php"/".phar".
As I showed, this is not true. I am kind of tired of repeating the same
thing again and again and being completely ignored, so unless there's
some new argument I'll stop repeating myself now.
Stas Malyshev
smalyshev@gmail.com
Hi Stas,
On Wed, Feb 25, 2015 at 9:52 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:
- require/include only includes ".php" ".phar" by default.
This is not true. As I repeatedly point out, your change only requires
that the string passed to include would end in .php, but string passed
to include and filename on filesystem are very different things, they do
not have to be the same at all!
- 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.I am at loss how you can claim it is "impossible" after I just
demonstrated how it is possible.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.
You sincerely do not understand why breaking all code that uses non-php
extensions is a problem and that editing existing production code's
entry points is by no way "simple"? That finding all extensions that may
be used for files in a complex software is very far from "simple"?Unlike previous RFC, this RFC patch does not care "contents" of file, but
only cares file "extensions". However, it works well as described before.It doesn't work well - in fact, it does not work at all, as I just
demonstrated, exactly because extension of the actual file and what
you're checking are different things.Even with this RFC, users can shoot their own foot and PHP is much easier
to do it wrong compare to other languages.These platitudes do not change the simple fact that your RFC does not do
what it claims to do, security-wise.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.No, it does not become safer, because if you have code that allows
script inclusion injection (and if you do, it's your fault, not PHP's)
it is trivial to exploit it even with this RFC.
- require/include cannot load script other than ".php"/".phar".
As I showed, this is not true. I am kind of tired of repeating the same
thing again and again and being completely ignored, so unless there's
some new argument I'll stop repeating myself now.
I think you misunderstood the RFC.
You seems forgetting about "allow_url_include=Off" by default.
Are you saying current PHP allows
include('zip://...') or include('input://...')?
Then this is serious bug. I'll fix it also.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Are you saying current PHP allows
include('zip://...') or include('input://...')?
Correction. include('input://..') should be include('php://input') (and
like)
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi!
Are you saying current PHP allows
include('zip://...') or include('input://...')?
Neither zip not phar are classified as url handlers. Both have is_url to 0.
Then this is serious bug. I'll fix it also.
This would be another big BC break, as this would mean you can not use
phar streams with allow_url_fopen set to off. Please don't change that,
there's reason for these settings.
--
Stas Malyshev
smalyshev@gmail.com
Hi Stas,
On Wed, Feb 25, 2015 at 12:19 PM, Stanislav Malyshev smalyshev@gmail.com
wrote:
Are you saying current PHP allows
include('zip://...') or include('input://...')?Neither zip not phar are classified as url handlers. Both have is_url to 0.
Then this is serious bug. I'll fix it also.
This would be another big BC break, as this would mean you can not use
phar streams with allow_url_fopen set to off. Please don't change that,
there's reason for these settings.
I have to at least php://
php://input or php://stdin
allows attacker script execution via POST if it's allowed
by allow_url_include=On.
[yohgaki@dev php-src]$ php -d allow_url_include=On -r
'include("php://input");' 2> /dev/null
[yohgaki@dev php-src]$
No errors. It seems we are better to fix this even with this RFC. Default
setting for web SAPI
prevents attack, but it can be disabled.
Other than this, it seems it's working as it should. (allow_url_include=Off)
[yohgaki@dev php-src]$ php -r 'include("php://input");' 2> /dev/null
Warning: include(php://input): failed to open stream: operation failed in
Command line code on line 1
Warning: include(): Failed opening 'php://input' for inclusion
(include_path='.:/usr/share/pear:/usr/share/php') in Command line code on
line 1
[yohgaki@dev php-src]$ php -r 'include("http://php.net";);' 2> /dev/null
Warning: include(): http:// wrapper is disabled in the server configuration
by allow_url_include=0 in Command line code on line 1
Warning: include(http://php.net): failed to open stream: no suitable
wrapper could be found in Command line code on line 1
Warning: include(): Failed opening 'http://php.net' for inclusion
(include_path='.:/usr/share/pear:/usr/share/php') in Command line code on
line 1
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi!
I have to at least php://
php://input or php://stdin
allows attacker script execution via POST if it's allowed
by allow_url_include=On.
allow_url_include=On means it's allowed. That's what "on" setting is
for. Production setting should always be "off".
Stas Malyshev
smalyshev@gmail.com
Here are my cents to this RFC, as it just keeps popping in in my inbox and its beginning to be one that I wish I could ignore.
First, … file extensions? A default Apache configuration and some Nginx configurations actually accept more than one file extension. This RFC does not include any way to specify a variety of extensions that should be blocked, ignored or treated else.
Your PHP code is only so secure as you make it. If you are in need for such an RFC just to block a few „rare cases“, then I would rather suggest you to either check your source or hand it to a professional to get it counter-checked.
Besides of that, it is never a good idea to let a user upload /everything/ that they want to. A proper MIME-type check can be helpful in these scenarios.
Again, I would not vote for the RFC and I do not think positive about it, since I see it very unnecessary.
Thus, if an attacker really wants to get into your business, they have more than one way to do so - for instance, exploiting the web server itself.
Kind regards,
Ingwie
Am 25.02.2015 um 05:57 schrieb Stanislav Malyshev smalyshev@gmail.com:
Hi!
I have to at least php://
php://input or php://stdin
allows attacker script execution via POST if it's allowed
by allow_url_include=On.allow_url_include=On means it's allowed. That's what "on" setting is
for. Production setting should always be "off".Stas Malyshev
smalyshev@gmail.com
Hi Kevin,
On Wed, Feb 25, 2015 at 5:18 PM, Kevin Ingwersen (Ingwie Phoenix) <
ingwie2000@googlemail.com> wrote:
Here are my cents to this RFC, as it just keeps popping in in my inbox and
its beginning to be one that I wish I could ignore.First, … file extensions? A default Apache configuration and some Nginx
configurations actually accept more than one file extension. This RFC does
not include any way to specify a variety of extensions that should be
blocked, ignored or treated else.
It's described in the implementation details section of the RFC.
This RFC do not address Web server configuration issues. Scripts
opened by Web servers are just executed as configured.
Your PHP code is only so secure as you make it. If you are in need for such
an RFC just to block a few „rare cases“, then I would rather suggest you to
either check your source or hand it to a professional to get it
counter-checked.Besides of that, it is never a good idea to let a user upload /everything/
that they want to. A proper MIME-type check can be helpful in these
scenarios.
MIME-type check cannot help at all as it does not guarantee no embedded PHP
scripts in it.
Even image resize nor removing exif info cannot help.
Without this RFC, single script inclusion vulnerability is enough to take
over victim server for most
systems.
Again, I would not vote for the RFC and I do not think positive about it,
since I see it very unnecessary.
Then, it means you misunderstood the issue here.
Thus, if an attacker really wants to get into your business, they have more
than one way to do so - for instance, exploiting the web server itself.
Exploiting PHP programs is much easier for attackers. That's the reason why
attackers check
vulnerable PHP programs. Check your web server access/error logs, you'll
see what I mean.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Kevin,
Your PHP code is only so secure as you make it. If you are in need for
such an RFC just to block a few „rare cases“, then I would rather suggest
you to either check your source or hand it to a professional to get it
counter-checked.Besides of that, it is never a good idea to let a user upload
/everything/ that they want to. A proper MIME-type check can be helpful in
these scenarios.MIME-type check cannot help at all as it does not guarantee no embedded
PHP scripts in it.
Even image resize nor removing exif info cannot help.
One more comment for this.
Do you know Ruby and Perl could be vulnerable to script inclusion if simple
image validation is used and there is script inclusion vulnerable code?
I'm not going to write how it could be done, because this is not a security
list.
Script inclusion can be done via image just like PHP with Ruby and PERL, yet
Ruby and PERL does not have vulnerable apps unlike PHP.
Why? Because it's much harder to attack with Ruby/PERL.
Please read
https://wiki.php.net/rfc/script_only_include#do_not_see_how_this_rfc_prevent_script_inclusion_attacks
this and if you ever see fatal issue, please let me know.
Thank you.
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Kevin,
On 25 February 2015 at 08:18, Kevin Ingwersen (Ingwie Phoenix)
ingwie2000@googlemail.com wrote:
Here are my cents to this RFC, as it just keeps popping in in my inbox and its beginning to be one that I wish I could ignore.
First, … file extensions? A default Apache configuration and some Nginx configurations actually accept more than one file extension. This RFC does not include any way to specify a variety of extensions that should be blocked, ignored or treated else.
Er...it has an ini setting to create the whitelist? Also, blacklists
would never work - there might as well be an infinite number of file
extensions.
Your PHP code is only so secure as you make it. If you are in need for such an RFC just to block a few „rare cases“, then I would rather suggest you to either check your source or hand it to a professional to get it counter-checked.
So it does block "rare cases"? I dislike assigning rarity to security
issues since that assumes we have actual hard data when we don't. All
I can say definitively is that certain exploits in the area have been
around since 2010. Most of the close-to-php reporting I've seen
arrives around 2012. However, playing a numbers game about whether to
implement a security protection or not is irrelevant. Either it is a
security issue or it is not. It is.
Getting your code audited is always a good idea, of course.
Besides of that, it is never a good idea to let a user upload /everything/ that they want to. A proper MIME-type check can be helpful in these scenarios.
PHP via EXIF jpeg field. MIME check would detect it as image/jpeg.
Crackers have been using it in the wilds for years. MIME check of
Stanislav's PHAR as a GIF would be detected as application/gzip for
comparison since it's not a valid image.
Again, I would not vote for the RFC and I do not think positive about it, since I see it very unnecessary.
Thus, if an attacker really wants to get into your business, they have more than one way to do so - for instance, exploiting the web server itself.
This last sentence is true enough. For some reason though, we still
fix other entirely unrelated security weaknesses in PHP itself...
Paddy
--
Pádraic Brady
http://blog.astrumfutura.com
http://www.survivethedeepend.com
Hi,
This RFC benefits may not be obvious for people on this list, but this
RFC eliminates certain type of "PHP malware". PHP's script inclusionI 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
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:
- Created file chump.php:
<?php
include $argv[1];
This is an idealized vulnerable script.
- Created file pwnd.php
<?php
echo "pwnd!";
This is an idealized exploit.
-
Put it into an archive:
tar cvzf cuteponies.gif pwnd.php -
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?
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
Hi Stas,
On Wed, Feb 25, 2015 at 7:07 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:
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:
- Created file chump.php:
<?php
include $argv[1];
This is an idealized vulnerable script.
- Created file pwnd.php
<?phpecho "pwnd!";
This is an idealized exploit.
Put it into an archive:
tar cvzf cuteponies.gif pwnd.phpRun 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
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
Hi Stas,
On Wed, Feb 25, 2015 at 7:31 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:
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.
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
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
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
Hi,
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:
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
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
Hi
On Wednesday, February 25, 2015, Stanislav Malyshev smalyshev@gmail.com
wrote:
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".
Well, you fire those right over and then we'll have a debate worth having
;).
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.
RFC does not target invalidated uploads. For heavens sake, it's a defense
in depth measure not a sentient AI!
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.
That is not true! The lack of validation is one of degrees. You are
speaking in absolutes and ignoring partial effectiveness. Your example had
ZERO validation. Yasuo's clearly targeted successful validation of an
image. Not none at all.
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 bymove_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.
No it doesn't! You are misrepresenting this RFC as a magic wand. That is
not the case and it is extremely frustrating to see you persist on this.
Read my emails and read Yasuo's and then Dan's. Then we can have some sort
of intelligible discussion.
Paddy
--
--
Pádraic Brady
http://blog.astrumfutura.com
http://www.survivethedeepend.com
Zend Framework Community Review Team
Zend Framework PHP-FIG Representative
Your example omitted the image validation step which would have
noticed your attempt to upload a phar immediately. Add that and try
again.
Image validation is no defense against this type of attack:
As soon as you have any possibility of including a file uploaded by an
attacker, you are probably going to lose.
cheers
Dan
Hi Dan
On 25 February 2015 at 00:09, Pádraic Brady <padraic.brady@gmail.com
javascript:;> wrote:Your example omitted the image validation step which would have
noticed your attempt to upload a phar immediately. Add that and try
again.Image validation is no defense against this type of attack:
As soon as you have any possibility of including a file uploaded by an
attacker, you are probably going to lose.
That was indeed my point as Yasuo has already explained earlier. Image
validation would however see a phar a mile off.
Paddy
--
--
Pádraic Brady
http://blog.astrumfutura.com
http://www.survivethedeepend.com
Zend Framework Community Review Team
Zend Framework PHP-FIG Representative
Hi!
That was indeed my point as Yasuo has already explained earlier. Image
validation would however see a phar a mile off.
How much would you bet against the possibility of a file existing that
can both pass as an image file of some type and as a valid zip or tgz or
tar file? Hint: don't go too high.
--
Stas Malyshev
smalyshev@gmail.com
Hi Dan,
On 25 February 2015 at 00:09, Pádraic Brady padraic.brady@gmail.com
wrote:Your example omitted the image validation step which would have
noticed your attempt to upload a phar immediately. Add that and try
again.Image validation is no defense against this type of attack:
As soon as you have any possibility of including a file uploaded by an
attacker, you are probably going to lose.
I know, and Padraic knows also, attacker can make image file
that cannot remove "embedded PHP script" even with image resize.
Even tool called "Image Fight" exists to fight against PHP script
embedded images.
I proposed to include/require to load specific file extensions, but I've
got many objections for the idea. Therefore, I've tried to "detect" embedded
"PHP script". However, it's complex and I cannot make sure there isn't
embedded "PHP script" in a file.
Current RFC is based on the original idea with additional
move_uploaded_file()
protection. It works well for the objective.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
As soon as you have any possibility of including a file uploaded by an
attacker, you are probably going to lose.
I think that this is perhaps the key here.
My framework for new sites requires a user to log in before they can
upload anything. So if your manager is worried about 'all this php
malware' and your system allow unmanaged uploads ... all bets are off.
The next thing I do with images is to create a thumbnail set, so only if
you can get at the original file will there be a problem. I admit that I
prefer to leave the file name unmanaged, but the option is to rename it
original.xxx is also available.
Anything uploaded that is not an image or can't be displayed as a
thumbnail gets displayed as an icon, and viewing code as text gets the
due diligence it deserves, so even if an approved user tries to add php
script it will not be placed in a location where even if they could
access it it could be run either directly or as an include file.
I totally understand the basis of the RFC, I just don't see that
creating a few more smoke and mirror obstacles to practices that are
perfectly safe when used correctly but will add more work to 'web
admins' who have to get around them even when their code is already safe?
The code injection example only works on the basis "Imagine a piece of
badly-written PHP code responsible for reading the image from disk and
sending it to the browser:" This change does nothing to fix the
badly-written code, but it is THAT which needs to be fixed rather than
perfectly safe systems that 'disobey' this nannying?
--
Lester Caine - G8HFL
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
Hi all,
As soon as you have any possibility of including a file uploaded by an
attacker, you are probably going to lose.I think that this is perhaps the key here.
I thought it's rather obvious how this RFC works, but apparently not.
I added following description to the RFC.
==============================================
Do not see how this RFC prevent script inclusion attacks
- include*()/require*() refuse to compile/execute file extensions other
than “.php .phar” by default. -
move_uploaded_file()refuse to move PHP script. “.php .phar” is refused
by default.
With this RFC, include*()/require*() only executes files have “.php” or
“.phar” extension and move_uploaded_file() refuse to move uploaded files
that can be executed as PHP script. Therefore, even most obvious mistake
like 'include $_GET[“var”];' will not work anymore. i.e. It cannot read
files like “include '/etc/passwd';” nor execute script like “include
'/path/to/upload/evil_image.jpg';”.
How could this RFC loose?
I'm not trying to protects users from shooting themselves.
However, this RFC protects PHP programs from script inclusion attack
as well as file inclusion attack via include/require by default.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
As soon as you have any possibility of including a file uploaded by an
attacker, you are probably going to lose.I think that this is perhaps the key here.
I thought it's rather obvious how this RFC works, but apparently not.
I added following description to the RFC.==============================================
Do not see how this RFC prevent script inclusion attacks
- include*()/require*() refuse to compile/execute file extensions other
than “.php .phar” by default.move_uploaded_file()refuse to move PHP script. “.php .phar” is refused
by default.With this RFC, include*()/require*() only executes files have “.php” or
“.phar” extension andmove_uploaded_file()refuse to move uploaded files
that can be executed as PHP script. Therefore, even most obvious mistake
like 'include $_GET[“var”];' will not work anymore. i.e. It cannot read
files like “include '/etc/passwd';” nor execute script like “include
'/path/to/upload/evil_image.jpg';”.How could this RFC loose?
I'm not trying to protects users from shooting themselves.
However, this RFC protects PHP programs from script inclusion attack
as well as file inclusion attack via include/require by default.
Totally understand what you are trying to do, and if the users you are
trying to protect actually downloaded PHP direct from the PHP site it
may stand a chance of actually doing that, but it's adding restrictions
that WILL break other distributions so either they have to re-work what
they do, or switch it off anyway. The people you are trying to protect
are going to be downloading a distribution that may well be using
'obvious mistakes' such as .inc or .php5 in addition to .php
There have been attempts in the past to make 'script only' files and and
these same sort of restrictions, but the fallout did prove more of a
problem. Example ... one of the legacy sites I just had to move stopped
doing any of the navigation stuff and would not send a contact email.
Was working fine previously ... but when I actually started looking at
the code strangely the pages were all .html ... yep ... a complex site
with lots of content which had originally been hand coded and at some
point a few little bits of php had been added in. My nginx setup had
disabled processing .html pages so broken site. I don't want to rename
all of the files to .php ... I don't need to ... so I've created a
php-fpm for .html only and we are working again. Only a few hours wasted
but the sort of thing we have to be able to cope with!
--
Lester Caine - G8HFL
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
Hi Lester,
Totally understand what you are trying to do, and if the users you are
trying to protect actually downloaded PHP direct from the PHP site it
may stand a chance of actually doing that, but it's adding restrictions
that WILL break other distributions so either they have to re-work what
they do, or switch it off anyway. The people you are trying to protect
are going to be downloading a distribution that may well be using
'obvious mistakes' such as .inc or .php5 in addition to .phpThere have been attempts in the past to make 'script only' files and and
these same sort of restrictions, but the fallout did prove more of a
problem. Example ... one of the legacy sites I just had to move stopped
doing any of the navigation stuff and would not send a contact email.
Was working fine previously ... but when I actually started looking at
the code strangely the pages were all .html ... yep ... a complex site
with lots of content which had originally been hand coded and at some
point a few little bits of php had been added in. My nginx setup had
disabled processing .html pages so broken site. I don't want to rename
all of the files to .php ... I don't need to ... so I've created a
php-fpm for .html only and we are working again. Only a few hours wasted
but the sort of thing we have to be able to cope with!
I understand people do all kinds of things.
Therefore, I'm allowing
ini_set('zend.script_extension', ''); // Disable protections at all.
It's users choice if they use systematically secure configuration or not.
However, providing systematically secure method/configuration is our
responsibility. IMHO.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
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
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
Hi Stas,
On Mon, Feb 23, 2015 at 5:02 PM, Stanislav Malyshev smalyshev@gmail.com
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
yohgaki@ohgaki.net