PHP processing a truncated POST request
@bukka is already working on this.
Since Oct 13, 2023.
- Dominant language
- C
- Stars
- 40.4k
- Forks
- 8.1k
- Avg merge
- 2d 13h
- Merged PRs (30d)
- 96
Description
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.
The following code can reproduce the issue:
<?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')));
}
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
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
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.
php_error_docref(NULL, E_WARNING, "Actual POST length exceeds Content-Length, and exceeds " ZEND_LONG_FMT " bytes", SG(post_max_size));
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.
--- 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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.