Possible widespread PHP configuration issue - security risk

I simply do not have time for the next several days. I’m literally
working day and night on an app that I need ready by Monday.

Sure - I can update stuff. I only meant if you can spare some mins to
contribute to a best efforts config.

Our emails crossed - I will edit the media wiki entry to include an
exclusion for the /images/ dir also. For me at least this is then
“secure”.

Plus, I am probably the worst person to work on PHP issues as I firmly
believe PHP to be utter crap starting from its conception right down to
the last byte of its actual implementation. It tries my patience in
ways a toddler wired on espresso couldn’t.

I hear you there… Allowing PHP apps on the server keeps me awake at
night…

As an aside, my solution has been to use linux-vservers for each php
app. This was what led me to nginx to keep the memory usage of such a
system low. It’s super easy to segment apps though and gives an extra
amount of resilience to the installation

Not relevant to our thread though…

Cheers

Ed W

On 27/08/2010 18:05, Cliff W. wrote:

Nevertheless, I’ve updated the MediaWiki entry.

I’m still having problems getting to the wiki - no .js files are loading
which is causing some wierd stuff to happen.

However, my opinion is that just adding try_files is only a partial
fix. If some way is found to upload .php files (bad wikipedia config)
or some other exploit is found that can bypass the try_files then we
still have an issue.

My mediawiki config does this:

             location ~ .*.php$ {
                     include /etc/nginx/fastcgi_params;
                     if ( $uri !~ "^/images/") {
                             fastcgi_pass    localhost:9000;
                     }
             }

Others have already pointed out that we can do better than my IF.
However, your try_files, plus the explicit exclusion of the /images/ dir
go a long way to secure mediawiki. Also I think the specific exclusion
of the /images/ dir becomes quite self-documenting, whereas the
try_files is quite a subtle fix?

Cheers

Ed W

On Fri, Aug 27, 2010 at 06:48:12PM +0100, Ed W wrote:

However, your try_files, plus the explicit exclusion of the /images/
dir go a long way to secure mediawiki. Also I think the specific
exclusion of the /images/ dir becomes quite self-documenting,
whereas the try_files is quite a subtle fix?

Why don’t you just check if the file exists?

I use something like that:

location ~ .php$ {
if (!-f $request_filename) {
return 404;
}
fastcgi_pass unix:/var/run/php-fpm/php-fpm.sock;
fastcgi_param SCRIPT_FILENAME $vpath/$fastcgi_script_name;
include fastcgi_params;
}

And it seems to fix the nginx issue.

Cheers

Ed W

---

nginx mailing list
[email protected]
nginx Info Page

–
ubitux

On Fri, 2010-08-27 at 19:52 +0200, ubitux wrote:

then we still have an issue.
Others have already pointed out that we can do better than my IF.
location ~ .php$ {
if (!-f $request_filename) {
return 404;
}
fastcgi_pass unix:/var/run/php-fpm/php-fpm.sock;
fastcgi_param SCRIPT_FILENAME $vpath/$fastcgi_script_name;
include fastcgi_params;
}

That’s exactly equivalent to the try_files, except longer and using a
deprecated feature.

Cliff

On Fri, Aug 27, 2010 at 11:13 AM, Cliff W. [email protected] wrote:

It is subtle, but all fixes are, because the underlying vulnerability is
quite subtle. Â What user isn’t going to look at that and say to
themselves “why do I need this if statement?”. Â Just use the try_files
and add a comment to its purpose.

The caveat with try_files is it means nginx has filesystem access to
check the existence of the file and an additional stat call (or more)

• it can be in the open file cache, modern systems it’s not a huge
  deal, etc, etc.

But it won’t help if you’re fastcgi_pass to a remote server that nginx
does not have the same path to the file (or have access to the php
file) at all.

On Fri, 2010-08-27 at 11:15 -0700, Michael S. wrote:

deal, etc, etc.

But it won’t help if you’re fastcgi_pass to a remote server that nginx
does not have the same path to the file (or have access to the php
file) at all.

Good point. I do prefer your more general fix, although I’d like
confirmation that it does fully address the issue (the whole split_path
thing is too weird for me to want to try to understand).

Regards,
Cliff

On Fri, 2010-08-27 at 18:48 +0100, Ed W wrote:

On 27/08/2010 18:05, Cliff W. wrote:

Nevertheless, I’ve updated the MediaWiki entry.

I’m still having problems getting to the wiki - no .js files are loading
which is causing some wierd stuff to happen.

I do not see any issues… does anyone else have a problem?

However, my opinion is that just adding try_files is only a partial
fix. If some way is found to upload .php files (bad wikipedia config)

I don’t think we should try to overcome people’s intentional
configuration. Not only is it completely their fault if they go
through the difficulty of enabling a feature that is off by default, but
now we are attempting to impose our will on a user who has made a
specific decision. This is always a bad road in software.

or some other exploit is found that can bypass the try_files then we
still have an issue.

Not going to worry about such speculative future issues. If such an
issue arises it will need to be addressed as an Nginx patch, not a
configuration option.

Others have already pointed out that we can do better than my IF.
However, your try_files, plus the explicit exclusion of the /images/ dir
go a long way to secure mediawiki. Also I think the specific exclusion
of the /images/ dir becomes quite self-documenting, whereas the
try_files is quite a subtle fix?

It is subtle, but all fixes are, because the underlying vulnerability is
quite subtle. What user isn’t going to look at that and say to
themselves “why do I need this if statement?”. Just use the try_files
and add a comment to its purpose.

Cliff

On 27/08/2010 16:45, Jim O. wrote:

It doesn’t work on the apps I mentioned. It simply won’t upload.
The apps you mentioned were vBulletin and IPB. I have done a little
more research on this and I believe I can smuggle in the PHP using jpeg
comments. The resulting file should pass all tests as a valid JPG, but
still be executable to the PHP interpreter…

The point is: my expectation is that with a bit of wriggling it should
be possible to find something which should get past your image upload
checks, but your PHP interpreter will still happily process it. If your
server is misconfigured to allow accidental execution of such files then
I think you have a gaping hole in your security… Bottom line is to
completely disable execution of all untrusted files (variety of ways
to do that of course)

Personally I don’t believe in trusting only to the upload filtering to
secure a web application. There is simply too much subtlety here that a
well crafted file should eventually be able to bypass…

Ed W

Let’s stop debating and start with a clean fix. It sounds like this is
all that is needed. Anyone want to verify?

php config:
cgi.fix_pathinfo=0

then just make sure nginx splits the path info for you in case your
app needs it with fastcgi_split_path_info:
location ~ .php$ {
fastcgi_pass 127.0.0.1:11000;
include fastcgi_params;
fastcgi_split_path_info ^(.+.php)(.*)$; # just throw this in
fastcgi_params too, then!
}

Is this the right solution? Yes or no?

On Fri, Aug 27, 2010 at 11:06:00AM -0700, Michael S. wrote:

include fastcgi_params;
fastcgi_split_path_info ^(.+.php)(.*)$; # just throw this in
fastcgi_params too, then!
}

Is this the right solution? Yes or no?

• location ~ .php$ {

• location ~ .php {

BTW, in 0.8.x you may use

location ~ ^(?.+.php)(?<path_info>.*)$ {
fastcgi_pass 127.0.0.1:11000;
fastcgi_param SCRIPT_FILENAME $script;
fastcgi_param PATH_INFO $path_info;
include fastcgi_params;
}

–
Igor S.
http://sysoev.ru/en/

On Fri, Aug 27, 2010 at 11:41:38AM -0700, Michael S. wrote:

On Fri, Aug 27, 2010 at 11:39 AM, Igor S. [email protected] wrote:

šlocation ~ ^(?.+.php)(?<path_info>.*)$ {
š šfastcgi_pass 127.0.0.1:11000;
š šfastcgi_param š SCRIPT_FILENAME š$script;

Doesn’t this typically have the $document_root$fastcgi_script_name -
so the full system path?

You are right:

š šfastcgi_param š SCRIPT_FILENAME š/path/to/files$script;

or

š šfastcgi_param š SCRIPT_FILENAME š$document_root$script;

Thanks for the pointers, though.

I will begin adopting this style once I check it quick and pushing it
on everyone I know…

This way saves one regex execution.
BTW, it’s better for perfomance and configuration maintenance reasons
to isolate regex locaitons inside static ones as Maxim has shown:

location / {
š location ~ ^(?.+.php)(?<path_info>.*)$ {
…
}
…
}

location /dir1/ {
…
}

location /dir2/ {
š location ~ ^(?.+.php)(?<path_info>.*)$ {
…
}
…
}

–
Igor S.
http://sysoev.ru/en/

On Fri, Aug 27, 2010 at 11:39 AM, Igor S. [email protected] wrote:

 location ~ ^(?.+.php)(?<path_info>.*)$ {
  fastcgi_pass 127.0.0.1:11000;
  fastcgi_param  SCRIPT_FILENAME  $script;

Doesn’t this typically have the $document_root$fastcgi_script_name -
so the full system path?

Thanks for the pointers, though.

I will begin adopting this style once I check it quick and pushing it
on everyone I know…

Initial testing shows:

cgi.fix_pathinfo = 0

and Igor’s suggestion:

location ~ ^(?.+.php)(?<path_info>.*)$ {
  fastcgi_pass 127.0.0.1:11000;
  fastcgi_param  SCRIPT_FILENAME  $document_root$script;
  fastcgi_param  PATH_INFO     $path_info;
  include fastcgi_params;
}

To be working properly. I need to check out PATH_INFO using old style
and new style, make sure it still reports the expected behavior for
PHP scripts (PATH_INFO, PHP_SELF, all that jazz)

The one thing I don’t like is now I have to hardcode that into each
place, unless I defined the fastcgi_pass location, and then just had a
php.conf - then all of this could be done with a single line of config
code.

set $fastcgi_pass = ‘127.0.0.1:11000’;
include php.conf;

php.conf would have this:

location ~ ^(?.+.php)(?<path_info>.*)$ {
  fastcgi_pass $fastcgi_pass;
  fastcgi_param  SCRIPT_FILENAME  $document_root$script;
  fastcgi_param  PATH_INFO     $path_info;
  include fastcgi_params;
}

Would that be a workable solution Igor? Prior to this new style of PHP
handling I used to only need two lines:

fastcgi_pass 127.0.0.1:11000;
include fastcgi_params;

On 27/08/2010 20:06, Michael S. wrote:

 include fastcgi_params;

}

To be working properly. I need to check out PATH_INFO using old style
and new style, make sure it still reports the expected behavior for
PHP scripts (PATH_INFO, PHP_SELF, all that jazz)

I will believe you that this works, but it seems incredibly subtle and I
for one don’t quite understand why it’s working?

My point is only that we need to document how/why this is the solution
or users will deviate (innocently) and re-introduce the problem

Please, please also lets have the final solution also include an example
of how to efficiently exclude a certain directory. A good proportion
of apps that I have seen, ship with example Apache configurations that
do exactly this.

The suggestion was adding an excluded location:

 location ^~/images/  {
     # just handle as static, don't consult regexps
 }

However, the issue I see here is that the user then needs to
specifically configure how that location should be handled, rather than
it being an exclusion to the original location

Can we work excluded locations into the regexp (negative lookahead
supported?):

location ~ ^(?!/images/)(?.+.php)(?<path_info>.*)$ {

The justification for excluding specific locations from the PHP
interpretor is that most applications don’t encourage uploaded
executable cgi scripts and better apps are going to ship with a
recommendation to disable script execution in the upload directory.
Actually the situation is worse on Apache because there are so many ways
to trick the interpreter to run files. However, it’s seems to be such
standard practice on Apache that it seems prudent to include it in our
standardised solution?

Lots of justifications for this via google:

http://www.google.co.uk/search?hl=en&q=disable+php+script+execution+upload

Someone argued that this might be wanted by the application… (Some
apps wants users to upload .php files which should then be
executable…!!!). I claim that it will be a serious minority of
applications desire this, and the vast majority will want uploaded files
to be non executable (regardless of extension)

Can we please, please, please try and make sure the recommended
configuration includes examples of specifically excluding locations
not expected to contain executable scripts… My proposal above…

Ed W

On Sat, Aug 28, 2010 at 3:14 AM, Ed W [email protected] wrote:

I will believe you that this works, but it seems incredibly subtle and I for
one don’t quite understand why it’s working?

My point is only that we need to document how/why this is the solution or
users will deviate (innocently) and re-introduce the problem

It is a bit more complex to drop in and not as “straightforward” as
one might hope. At the moment I have this working:

main nginx.conf in a server {} block:

set $fastcgi 127.0.0.1:11000;
include confs/php.conf;

root@local:/etc/nginx# cat confs/php.conf
location ~ ^(?.+.php)(?<path_info>.*)$ {
fastcgi_buffers 16 8k;
fastcgi_buffer_size 8k;
fastcgi_busy_buffers_size 16k;
fastcgi_ignore_client_abort on;
fastcgi_index index.php;
fastcgi_intercept_errors on;
fastcgi_param CONTENT_LENGTH $content_length;
fastcgi_param CONTENT_TYPE $content_type;
fastcgi_param DOCUMENT_ROOT $document_root;
fastcgi_param DOCUMENT_URI $document_uri;
fastcgi_param GATEWAY_INTERFACE CGI/1.1;
fastcgi_param QUERY_STRING $query_string;
fastcgi_param PATH_INFO $path_info;
fastcgi_param REDIRECT_STATUS 200;
fastcgi_param REMOTE_ADDR $remote_addr;
fastcgi_param REMOTE_PORT $remote_port;
fastcgi_param REQUEST_METHOD $request_method;
fastcgi_param REQUEST_URI $request_uri;
fastcgi_param SCRIPT_FILENAME $document_root$script;
fastcgi_param SCRIPT_NAME $fastcgi_script_name;
fastcgi_param SERVER_ADDR $server_addr;
fastcgi_param SERVER_NAME $http_host;
fastcgi_param SERVER_PORT $server_port;
fastcgi_param SERVER_PROTOCOL $server_protocol;
fastcgi_param SERVER_SOFTWARE nginx/$nginx_version;
fastcgi_pass $fastcgi;
}

Now, it looks like $_SERVER[‘PATH_INFO’] is never filled in unless you
have /foo.php/somethingafterit

With cgi.fix_pathinfo=1, PATH_INFO = “/foo.php/somethingafterit”
With cgi.fix_pathinfo=0, PATH_INFO = “/somethingafterit”

Otherwise, PATH_INFO is empty if there is nothing after the .php.

PHP_SELF is empty using the new style approach to the nginx config
block.

Using the old style, $_SERVER[‘PHP_SELF’] works; I tried setting a
fastcgi_param for it, but it did not take. It seems like this is
derived internally in PHP and not able to be overridden.

A lot of things reference PHP_SELF, so this could introduce an issue.
It’s late, but my quick tests show a glaring caveat with that.

All I’m reading here is reiterations of the previous discussion and
language elites-ism .

This is an extremely old issue, the opinion last time was that this is
not something that should be fixed in Nginx. Nginx is a reverse proxy
and there may be very valid cases where allowing such URIs make sense.

The real solution is to fix the php pathinfo setting, it’s archaic and
shouldn’t be used unless absolutely necessary. That said, I did look
around a bit on the wiki and it wasn’t covered overly much, about the
only place was in my Nginx primer post so I’ll go ahead and add a
section on the pitfalls page that details the issue.

Posted at Nginx Forum: