|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2019-09-26 16:17 UTC] neex dot emil+phpeb at gmail dot com
Description: ------------ The line 1140 in file sapi/fpm/fpm/fpm_main.c (https://github.com/php/php-src/blob/master/sapi/fpm/fpm/fpm_main.c#L1140) contains pointer arithmetics that assumes that env_path_info has a prefix equal to the path to the php script. However, the code does not check this assumption is satisfied. The absence of the check can lead to an invalid pointer in the "path_info" variable. Such conditions can be achieved in a pretty standard Nginx configuration. If one has Nginx config like this: ``` location ~ [^/]\.php(/|$) { fastcgi_split_path_info ^(.+?\.php)(/.*)$; fastcgi_param PATH_INFO $fastcgi_path_info; fastcgi_pass php:9000; ... } } ``` The regexp in `fastcgi_split_path_info` directive can be broken using the newline character (in encoded form, %0a). Broken regexp leads to empty PATH_INFO, which triggers the bug. This issue leads to code execution. Later in the code, the value of path_info[0] is set to zero (https://github.com/php/php-src/blob/master/sapi/fpm/fpm/fpm_main.c#L1150); then FCGI_PUTENV is called. Using a carefully chosen length of the URL path and query string, an attacker can make path_info point precisely to the first byte of _fcgi_data_seg structure. Putting zero into it moves `char* pos` field backwards, and following FCGI_PUTENV overwrites some data (including other fast cgi variables) with the script path. Using this technique, I was able to create a fake PHP_VALUE fcgi variable and then use a chain of carefully chosen config values to get code execution. I have a working exploit PoC, but I'm not sure how to share it using this form. This security research is done by three people: me, @beched and @d90pwn. Test script: --------------- To reproduce the issue, you need to take the following steps: 1. Build php with --enable-fpm and ASAN enabled. 2. Download https://www.dropbox.com/s/eio9zikkg1juuj7/reproducer.tar.xz?dl=0. The following steps assume you're in the `reproducer` directory from the archive. 4. Run nginx using `sudo /usr/sbin/nginx -p $PWD -c nginx.conf` 3. Run php-fpm using `path/to/php-fpm -y ./php-fpm.conf -F` 4. Visit a (pretty long) link from crash_link.txt using a tool another tool, like curl $(cat crash_link.txt). Expected result: ---------------- No crash should happen. Actual result: -------------- You will get a crash: ==6629==ERROR: AddressSanitizer: SEGV on unknown address 0x620000005203 (pc 0x7efd1341a47f bp 0x7ffe980574e0 sp 0x7ffe98056c98 T0) ==6629==The signal is caused by a WRITE memory access. #0 0x7efd1341a47e in memcpy /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:140 #1 0x4b7c57 in __asan_memcpy /home/emil/llvm/projects/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 #2 0x13a88df in fcgi_hash_strndup /home/emil/php-src/main/fastcgi.c:322:2 #3 0x13a88df in fcgi_hash_set /home/emil/php-src/main/fastcgi.c:359:11 #4 0x13c4121 in init_request_info /home/emil/php-src/sapi/fpm/fpm/fpm_main.c:1154:12 #5 0x13c4121 in main /home/emil/php-src/sapi/fpm/fpm/fpm_main.c:1864:4 #6 0x7efd13380b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310 #7 0x440219 in _start (/home/emil/php-src/builded/sbin/php-fpm+0x440219) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:140 in memcpy Patches0001-Fix-bug-78599-env_path_info-underflow-can-lead-to-RC.patch (last revision 2019-10-12 14:59 UTC by bukka@php.net)initial-no-test (last revision 2019-10-06 18:24 UTC by bukka@php.net) Pull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||
Copyright © 2001-2020 The PHP GroupAll rights reserved. |
Last updated: Fri Feb 07 06:01:25 2020 UTC |
I have been looking into it and looks quite nasty. I think something like this (based on PHP-7.1 branch) should probably fix it: diff --git a/sapi/fpm/fpm/fpm_main.c b/sapi/fpm/fpm/fpm_main.c index 24a7e5d56a..50f92981f1 100644 --- a/sapi/fpm/fpm/fpm_main.c +++ b/sapi/fpm/fpm/fpm_main.c @@ -1209,8 +1209,8 @@ static void init_request_info(void) path_info = script_path_translated + ptlen; tflag = (slen != 0 && (!orig_path_info || strcmp(orig_path_info, path_info) != 0)); } else { - path_info = env_path_info ? env_path_info + pilen - slen : NULL; - tflag = (orig_path_info != path_info); + path_info = (env_path_info && pilen > slen) ? env_path_info + pilen - slen : NULL; + tflag = path_info && (orig_path_info != path_info); } if (tflag) { However it's just based on code logic. It means it's not really tested. I started looking to creating a test but need to figure out what exactly nginx sends. Tried with [ 'SCRIPT_FILENAME' => $uri, 'SCRIPT_NAME' => $uri . '/abcd', 'PATH_INFO' => '', ] but it didn't crash so will need to try it with nginx and gdb or wireshark to get all fcgi params. If anyone can get all fcgi params causing a crash, that would be really helpful as I don't have much free time...No, currently I don't have any ideas on making the test more reliable. However, I was able to reproduce the crash using the test suite --- hope it helps. The patch indeed fixes it. I'm using the following set of variables (did it by locally modifying tester.inc): [ 'GATEWAY_INTERFACE' => 'FastCGI/1.0', 'REQUEST_METHOD' => 'GET', 'SCRIPT_FILENAME' => $uri."/".str_repeat('A', 35), 'SCRIPT_NAME' => $uri, 'QUERY_STRING' => $query, 'REQUEST_URI' => $uri . ($query ? '?'.$query : ""), 'DOCUMENT_URI' => $uri, 'SERVER_SOFTWARE' => 'php/fcgiclient', 'REMOTE_ADDR' => '127.0.0.1', 'REMOTE_PORT' => '7777', 'SERVER_ADDR' => '127.0.0.1', 'SERVER_PORT' => '80', 'SERVER_NAME' => php_uname('n'), 'SERVER_PROTOCOL' => 'HTTP/1.1', 'DOCUMENT_ROOT' => __DIR__, 'CONTENT_TYPE' => '', 'CONTENT_LENGTH' => 0, 'HTTP_HUI' => str_repeat('PIZDA', 1000), 'PATH_INFO' => '', ], Changes are: SCRIPT_FILENAME has an appended suffix, HTTP_HUI and PATH_INFO have been added. Things to note: 1. There was a mistake in all recent comments; the path_info should be appended to SCRIPT_FILENAME, not to SCRIPT_NAME. 2. There's a "request header" above right before PATH_INFO. The length of its' value is long enough to make fcgi_hash_strndup create new _fcgi_data_seg both for the said value and the following PATH_INFO (name and value). 3. The path info (which is appended to SCRIPT_FILENAME) is exactly 35 bytes long. This number is calculated as follows: size of _fcgi_data_seg's header is 3 * 8 = 24 bytes, length of string "PATH_INFO" (which is the variable name and is also stored inside _fcgi_data_seg) is 9 bytes plus the final zero byte, and an additional byte is needed to make path_info variable point right before the _fcgi_data_seg to make ASAN scream. This test is still not very nice as it relies on the order of variables, length of _fcgi_data_seg, etc. However, if you just need a test that don't pass without the patch, it is it. The error message is "Not in white list. Check listen.allowed_clients.", but I believe it is always shown in case of incorrect FastCGI reply. It looks like to make the set of the variables above a regular test, we need to implement path_info support in Tester::request. Currently, it doesn't call makeFile if $uri is supplied and doesn't allow to append path_info otherwise.A method to quick fix this problem. If you want to use PATH_INFO in php, and do not want to patch and recompile PHP. Add this line before ALL YOUR "location ~ \.php(/|$) {" LINES in nginx confs: ``` rewrite ^(.*?)\n $1; #Fix CVE-2019-11043 (THIS LINE!!!) location ~ \.php(/|$) { ... fastcgi_split_path_info ^((?U).+\.php)(/?.+)$; ... ``` That will truncate PATH_INFO after "\n" while URL contains "%0a".There are better workarounds for this on the Nginx level than the one presented in an earlier comment. The simplest is to conditionally set PATH_INFO if it's not empty: fastcgi_param PATH_INFO $fastcgi_path_info if_not_empty; In my tests, that successfully prevents an attack. Another option is to explicitly test whether the FCGI script path exists; this is useful anyway because otherwise, PHP-FPM will produce the 404 error page, which isn't customizable: if (!-f $document_root$fastcgi_script_name) { # check if the script exists # otherwise, /foo.jpg/bar.php would get passed to FPM, which wouldn't run it as it's not in the list of allowed extensions, but this check is a good idea anyway, just in case return 404; }