php / php/php-src

PHP processing a truncated POST request

Đang mở
#12,343 5 bình luận 2 reaction 1 người được giao Xem trên GitHub

@bukka đang làm issue này rồi.

Từ ngày 13/10/2023.

Bug Category: SAPI SAPI: cgi SAPI: fpm
Ngôn ngữ chính
C
Star
40.4k
Fork
8.2k
Merge trung bình
2 ngày 13 giờ
Pull request đã merge (30 ngày)
96

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.