php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #73342 Vulnerability in php-fpm by changing stdin to non-blocking
Submitted: 2016-10-18 20:02 UTC Modified: 2018-06-20 11:18 UTC
Votes:22
Avg. Score:4.7 ± 0.6
Reproduced:21 of 21 (100.0%)
Same Version:7 (33.3%)
Same OS:5 (23.8%)
From: xuavis at gmail dot com Assigned: bukka (profile)
Status: Re-Opened Package: FPM related
PHP Version: 7.0Git-2016-10-18 (Git) OS: Ubuntu 16.04
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2016-10-18 20:02 UTC] xuavis at gmail dot com
Description:
------------
Tested on php 5.6 and 7.0 on both Ubuntu and FreeBSD.

When changing STDIN to non-blocking (see test script), php-fpm child processes start crashing.

This causes rapid restarting of child processes with high cpu load. This can be exploited for denial of service. Also scripts start getting cut off during execution.

Sometimes the master php-fpm process also stops responding to a restart/stop and requires a SIGKILL to stop.

Test script:
---------------
<?php

stream_set_blocking(fopen('php://stdin', 'r'), false);

Expected result:
----------------
strace of child:

write(3, "\1\6\0\1\0S\5\0X-Powered-By: PHP/5.6.27"..., 112) = 112
shutdown(3, SHUT_WR)                    = 0
recvfrom(3, "\1\5\0\1\0\0\0\0", 8, 0, NULL, NULL) = 8
recvfrom(3, "", 8, 0, NULL, NULL)       = 0
close(3)                                = 0
setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={0, 0}}, NULL) = 0
accept(0, strace: Process 19005 detached

clean php-fpm log

Actual result:
--------------
strace of child:

write(3, "\1\6\0\1\0S\5\0X-Powered-By: PHP/5.6.27"..., 112) = 112
shutdown(3, SHUT_WR)                    = 0
recvfrom(3, "\1\5\0\1\0\0\0\0", 8, 0, NULL, NULL) = 8
recvfrom(3, "", 8, 0, NULL, NULL)       = 0
close(3)                                = 0
setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={0, 0}}, NULL) = 0
accept(0, 0x7ffeb7a996c0, 0x7ffeb7a996b0) = -1 EAGAIN (Resource temporarily unavailable)

php-fpm log:

[17-Oct-2016 23:28:49.808690] NOTICE: pid 748, fpm_children_bury(), line 252: [pool www] child 6300 exited with code 0 after 0.001693 seconds from start
[17-Oct-2016 23:28:49.808927] NOTICE: pid 748, fpm_children_make(), line 421: [pool www] child 6301 started
[17-Oct-2016 23:28:49.808946] DEBUG: pid 748, fpm_event_loop(), line 419: event module triggered 1 events
[17-Oct-2016 23:28:49.810581] DEBUG: pid 748, fpm_got_signal(), line 76: received SIGCHLD
[17-Oct-2016 23:28:49.810611] NOTICE: pid 748, fpm_children_bury(), line 252: [pool www] child 6301 exited with code 0 after 0.001690 seconds from start
[17-Oct-2016 23:28:49.810833] NOTICE: pid 748, fpm_children_make(), line 421: [pool www] child 6302 started
[17-Oct-2016 23:28:49.810852] DEBUG: pid 748, fpm_event_loop(), line 419: event module triggered 1 events
[17-Oct-2016 23:28:49.812501] DEBUG: pid 748, fpm_got_signal(), line 76: received SIGCHLD
[17-Oct-2016 23:28:49.812533] NOTICE: pid 748, fpm_children_bury(), line 252: [pool www] child 6302 exited with code 0 after 0.001706 seconds from start
[17-Oct-2016 23:28:49.812750] NOTICE: pid 748, fpm_children_make(), line 421: [pool www] child 6303 started
[17-Oct-2016 23:28:49.812771] DEBUG: pid 748, fpm_event_loop(), line 419: event module triggered 1 events
[17-Oct-2016 23:28:49.814332] DEBUG: pid 748, fpm_got_signal(), line 76: received SIGCHLD
[17-Oct-2016 23:28:49.814362] NOTICE: pid 748, fpm_children_bury(), line 252: [pool www] child 6303 exited with code 0 after 0.001618 seconds from start
[17-Oct-2016 23:28:49.814585] NOTICE: pid 748, fpm_children_make(), line 421: [pool www] child 6304 started

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-10-18 20:16 UTC] xuavis at gmail dot com
The patch I think I added doesn't show for me. You can also find it at https://gist.github.com/anonymous/080bdb43fe9a0eb57609a93e9e4dd093
 [2016-10-19 05:33 UTC] stas@php.net
-Assigned To: +Assigned To: fat
 [2016-10-19 05:33 UTC] stas@php.net
Not sure how it's security issue. If you can run code on this server, you have already crossed security border.
 [2016-10-19 05:37 UTC] krakjoe@php.net
Agree with stas, not a sec issue.
 [2016-10-19 07:36 UTC] xuavis at gmail dot com
The case I could think of (and the reason I did choose Security as type) was due to shared hosting providers; Running multiple users on the same server where a single user can, as far as I know, (almost) undetectable use this to cause high load leading to a Denial of Service which affects other users.
 [2016-10-19 19:03 UTC] stas@php.net
-Type: Security +Type: Bug
 [2016-10-25 17:25 UTC] cmb@php.net
-Status: Assigned +Status: Open -Assigned To: fat +Assigned To:
 [2016-10-25 17:25 UTC] cmb@php.net
Unassigning from Jérôme, since his latest commit has been more
than 4 years ago. Hopefully, somebody else will tackle this issue
this way.
 [2017-05-16 09:05 UTC] yago dot riveiro at gmail dot com
This bug with Centos 7.0 and php 7 can be reproduced too
 [2017-07-16 17:15 UTC] matt at datacodesolutions dot com
This bug is causing issues on our server (Centos 7 / WHM / PHP 70) as well - high load which in turn cause extremely slow performance.  We use shell_exec to generate PDFs and it seems like this is causing the PHP-FPM service to get stuck in an endless loop of trying to start and then exiting.

https://serverfault.com/questions/839135/nginx-php-fpm-child-exited-with-code-0

[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62666 exited with code 0 after 0.103145 seconds from start
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62675 started
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62665 exited with code 0 after 0.108670 seconds from start
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62676 started
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62667 exited with code 0 after 0.111453 seconds from start
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62677 started
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62668 exited with code 0 after 0.112478 seconds from start
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62678 started
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62669 exited with code 0 after 0.098957 seconds from start
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62679 started
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62671 exited with code 0 after 0.094954 seconds from start
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62680 started
[16-Jul-2017 12:49:45] NOTICE: [pool mysite_com] child 62670 exited with code 0 after 0.101192 seconds from start
 [2018-02-23 17:01 UTC] nikic@php.net
-Status: Open +Status: Verified
 [2018-02-23 18:26 UTC] stas@php.net
-Assigned To: +Assigned To: bukka
 [2018-02-23 20:41 UTC] nikic@php.net
I think the patch from the first comment only works around the problem ... the real question is this: Why does FPM care about STDIN at all?

After some looking around, this seems to be what happens:
 * wp->listening_socket is what we actually want to listen on.
 * fpm_globals.listening_socket is always 0 (effectively STDIN), because that's what the global is initialized to. It's never changed.
 * fpm_run() always returns fpm_globals.listening_socket and that's what fcgi listens on.
 * to make things line up fpm_stdio_init_child() does a dup2(wp->listening_socket, STDIN_FILENO).

So effectively we take the listening socket, dup2 it to STDIN and then listen on STDIN. The following patch removes the indirection through STDIN:

diff --git a/sapi/fpm/fpm/fpm_children.c b/sapi/fpm/fpm/fpm_children.c
index b48fa54..4ee316b 100644
--- a/sapi/fpm/fpm/fpm_children.c
+++ b/sapi/fpm/fpm/fpm_children.c
@@ -146,6 +146,7 @@ static struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */
 static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
 {
 	fpm_globals.max_requests = wp->config->pm_max_requests;
+	fpm_globals.listening_socket = dup(wp->listening_socket);
 
 	if (0 > fpm_stdio_init_child(wp)  ||
 	    0 > fpm_log_init_child(wp)    ||
diff --git a/sapi/fpm/fpm/fpm_stdio.c b/sapi/fpm/fpm/fpm_stdio.c
index 4072017..76e8b32 100644
--- a/sapi/fpm/fpm/fpm_stdio.c
+++ b/sapi/fpm/fpm/fpm_stdio.c
@@ -103,12 +103,6 @@ int fpm_stdio_init_child(struct fpm_worker_pool_s *wp) /* {{{ */
 	fpm_globals.error_log_fd = -1;
 	zlog_set_fd(-1);
 
-	if (wp->listening_socket != STDIN_FILENO) {
-		if (0 > dup2(wp->listening_socket, STDIN_FILENO)) {
-			zlog(ZLOG_SYSERROR, "failed to init child stdio: dup2()");
-			return -1;
-		}
-	}
 	return 0;
 }
 /* }}} */

This also resolves the issue for me. However, I don't know if this has any side-effects, because something else relies on the STDIN mapping.
 [2018-02-25 19:21 UTC] bukka@php.net
@nikic I think that the reason why FPM cares about STDIN is to conform FastCGI spec - namely section 2.2 ( http://www.mit.edu/~yandros/doc/specs/fcgi-spec.html#S2.2 ) that states "FCGI_LISTENSOCK_FILENO equals STDIN_FILENO".

I guess it's because maybe some FastCGI application could access the data directly from the record but not really sure why. I can't really see the reason for that in the PHP case. I don't see any side effects of the patch at the moment but it might need a bit more thinking and testing.
 [2018-03-07 09:22 UTC] kenny at kennynet dot co dot uk
We've deployed the suggested patch removing the use of STDIN internally (currently in testing pending production deployment). I'll update this bug report if we see any problems caused by it.
 [2018-04-24 06:10 UTC] gksalil at gmail dot com
Hello

   Is the patch working and fix the issue ? Can I apply this patch safely ?

Thanks
~S
 [2018-06-11 12:39 UTC] alex at sirensclef dot com
I'm under some pressure to put together a production PHP 7.2 + FPM setup, but then I ran head first into this bug. I don't know what's worse... that a critical subset of PHP functionality is incompatible with PHP-FPM, or that this has been known for so long and nothing has been done.

Is anyone aware of a workaround (other than this patch being evaluated)? I've got more of a mod_php background so I'm only just developing an understanding of the options here, but I've had no luck making adjustments to neutralize the issue. I've tested an option, for a dev server, that uses a cron to check the php-fpm log every minute and restart the service whenever the issue arises, but I can't put that into production. Worried this setup is simply a non-starter.
 [2018-06-11 13:06 UTC] nikic@php.net
@bukka: What's the ETA on getting this merged? I saw that you submitted the PR for the fpm test revamp, so I assume that we'll start landing outstanding fpm patches soon?
 [2018-06-12 09:03 UTC] kenny at kennynet dot co dot uk
We've seen no problems yet with the proposed patch in our internal infrastructure and we've applied it against both 7.0 and 7.2.
 [2018-06-12 15:47 UTC] bukka@php.net
@nikic Yep that's sort of the plan. I want to finish the fpm logging changes and then will try to look on this one and other stuff. Can't really give you an ETA but hopefully it won't take too long. Will see. ;)
 [2018-06-12 16:34 UTC] nikic@php.net
@bukka: Thanks for the update. That sounds like things might still take a while, in which case I will go ahead and merge this patch now. We can't afford to delay this issue by an indeterminate amount of time, and given the production testing by kenny at kennynet dot co dot uk, I believe we can be sufficiently confident in the correctness of the change.
 [2018-06-12 16:51 UTC] bukka@php.net
@nikic Please can you create a PR first. I will try to look at it this week and if I don't find a time, go ahead but I think I should have time. Also a test should be present - that should be quite easy to do in the new tests...
 [2018-06-12 16:55 UTC] bukka@php.net
Btw. I would a bit careful as we are breaking FastCGI "spec" (it's not really a spec though and it doesn't seem to be a useful part anywya) so maybe we should just target 7.2 first and then possibly back port it. WDT?
 [2018-06-12 17:34 UTC] bukka@php.net
I just merged the test revamp. Think that the test for this could be similar to the one for fastcgi_finish_request - https://git.io/vhrAj . Of course the PHP code will be different (as test script) and there should be probably two requests and possibly some other changes - not exactly sure atm. as I would have to try it but think you will get the idea ;)
 [2018-06-12 17:45 UTC] nikic@php.net
@bukka: I can't remember writing it, but I have this test against the old infrastructure lying around: https://gist.github.com/nikic/5904446746a7f90f98e856e6dda592ee I'll port it and submit a PR.
 [2018-06-12 18:42 UTC] nikic@php.net
@bukka: PR up at https://github.com/php/php-src/pull/3287.
 [2018-06-20 10:22 UTC] DEBANANDA dot PAL at GMAIL dot COM
Hi

Our current php5 version is 5.6.34.
Pls let me know, if we can apply the patch (described in below link) to php5 5.6.34 

https://bugs.php.net/bug.php?id=73342&thanks=3

Thanks,
D Pal
 [2018-06-20 10:33 UTC] nikic@php.net
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=69dee5c732fe982c82edb17d0dbc3e79a47748d8
Log: Fixed bug #73342
 [2018-06-20 10:33 UTC] nikic@php.net
-Status: Verified +Status: Closed
 [2018-06-20 10:37 UTC] nikic@php.net
-Status: Closed +Status: Re-Opened -Type: Bug +Type: Security -Private report: No +Private report: Yes
 [2018-06-20 10:37 UTC] nikic@php.net
Reopening with security classification to track backports to PHP 5.6 and PHP 7.0.
 [2018-06-20 11:18 UTC] bukka@php.net
I don't think this is a security issue. It can cause issue just for the same pool running under same user. Basically it can't leak any information from the different pool. That actually reminds me https://bugs.php.net/bug.php?id=76067&edit=1 that had similar discussion and should be also changed to just a bug.
 [2018-07-06 09:20 UTC] zoolyka at gmail dot com
The issue still exists (7.2.7-1+ubuntu16.04.1+deb.sury.org+1), and its very annoying. As a workaround append " &>/dev/null" to the executed command.
 [2018-07-06 17:40 UTC] zoolyka at gmail dot com
Sorry, that "&>/dev/null" did not worked out. Using the following method instead of exec, shell_exec, etc. prevents the crashing loop:

$command = 'command to execute...';
$pipes = array();
$process = proc_open(
  $command,
  array(
     0 => array('pipe', 'r'),
     1 => array('pipe', 'w'),
     2 => array('pipe', 'w'),
  ),
  $pipes
);

stream_set_blocking($pipes[1], true);
stream_set_blocking($pipes[2], true);

if (
  is_resource($process)
) {

  $output = stream_get_contents($pipes[1]);
  $error = stream_get_contents($pipes[2]);

  fclose($pipes[1]);
  fclose($pipes[2]);

  proc_close($process);

}
 
PHP Copyright © 2001-2018 The PHP Group
All rights reserved.
Last updated: Mon Nov 19 05:01:26 2018 UTC