Bug #73342 [Com]: Vulnerability in php-fpm by changing stdin to non-blocking

Edit report at https://bugs.php.net/bug.php?id=73342&edit=1 ID: 73342 Comment by: kenny at kennynet dot co dot uk Reported by: xuavis at gmail dot com Summary: Vulnerability in php-fpm by changing stdin to non-blocking Status: Verified Type: Bug Package: FPM related Operating System: Ubuntu 16.04 PHP Version: 7.0Git-2016-10-18 (Git) Assigned To: bukka Block user comment: N Private report: N New Comment: 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. Previous Comments: ------------------------------------------------------------------------ [2018-02-25 19:21:00] 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-02-23 21:10:52] nikic@php.net Related To: Bug #67796 ------------------------------------------------------------------------ [2018-02-23 20:48:52] nikic@php.net Related To: Bug #73056 ------------------------------------------------------------------------ [2018-02-23 20:41:40] 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-23 16:55:15] nikic@php.net Related To: Bug #70185 ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=73342 -- Edit this bug report at https://bugs.php.net/bug.php?id=73342&edit=1