[php-src] Issue #12343: PHP processing a truncated POST request

From: Date: Mon, 02 Oct 2023 10:03:45 +0000
Subject: [php-src] Issue #12343: PHP processing a truncated POST request
Groups: php.bugs 
Request: Send a blank email to php-bugs+get-245475@lists.php.net to get a copy of this message
Issue: https://github.com/php/php-src/issues/12343 Author: ScottHelme ### Description I've observed scenarios where PHP will read only a partial POST payload and begin processing it. The main scenario seems to be when Nginx times out on the fastcgi_pass to PHP, PHP will read whatever partial payload was written to the socket buffer and process it. <br> The following code can reproduce the issue: ```php <?php sleep(30); $json = json_decode(file_get_contents('php://input')); if ($json == null) { syslog(LOG_WARNING, "JSON decode failed: " . $_SERVER['CONTENT_LENGTH'] . " " . strlen(file_get_contents('php://input'))); } ``` <br> Send enough requests to exceed pm.max_children and cause a backlog. You can modify fastcgi_connect_timeout and fastcgi_send_timeout to a lower value than default to exacerbate the issue for testing. I believe the issue is in the read() system call, which is returning 0 before expected, but PHP does not check the size of the POST payload that was read against the declared content-length. https://github.com/php/php-src/blob/1c93cdcea430b77a28b4a552a350cd84484f7259/main/fastcgi.c#L970 <br> The only validation of the size of the POST payload is in SAPI_POST_READER_FUNC(). This function initially checks the declared content-length does not exceed post_max_size, then reads the POST payload, and then checks the actual size of the POST payload that was read does not exceed post_max_size here. https://github.com/php/php-src/blob/1c93cdcea430b77a28b4a552a350cd84484f7259/main/SAPI.c#L282-L285 <br> The error message is also misleading and could be updated. It implies that the check could detect a POST length longer or shorter than the declared content-length with "does not match", but actually it will only detect a scenario where the POST length is longer than declared. An error message like this would be more accurate. ```c php_error_docref(NULL, E_WARNING, "Actual POST length exceeds Content-Length, and exceeds " ZEND_LONG_FMT " bytes", SG(post_max_size)); ``` <br> I created a patch for SAPI.c to demonstrate a possible behaviour and handling of the truncated POST payload that I might expect to see. This will check if the size of the POST payload read was smaller than the size declared in content-length and discard it if so. ```bash --- SAPI.c.orig 2023-09-29 12:32:46.020740222 +0000 +++ SAPI.c 2023-09-29 20:28:36.530234632 +0000 @@ -288,6 +288,12 @@ break; } } + + if (SG(read_post_bytes) < SG(request_info).content_length) { + php_stream_truncate_set_size(SG(request_info).request_body, 0); + php_error_docref(NULL, E_WARNING, "POST length of " ZEND_LONG_FMT " bytes was less than declared Content-Length " ZEND_LONG_FMT " bytes; all data discarded", SG(read_post_bytes), SG(request_info).content_length); + } + php_stream_rewind(SG(request_info).request_body); } } ``` ### PHP Version 8.2.10 ### Operating System Ubuntu 22.04 (LTS) x64

« previous php.bugs (#245475) next »