php / php/php-src

PHP processing a truncated POST request

Abierto
#12,343 5 comentarios 2 reacciones 1 asignado Ver en GitHub

@bukka ya está trabajando en esto.

Desde el 13/10/2023.

Bug Category: SAPI SAPI: cgi SAPI: fpm
Lenguaje dominante
C
Estrellas
40.4k
Forks
8.2k
Merge medio
2 d 13 h
PR fusionados (30 d)
96

Descripción

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

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.