From 939ac48d908f7845c173be0be875e1b100cdacb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 10 Nov 2026 17:09:10 +0100 Subject: [PATCH 1/2] Fix uploaded files leak in request_parse_body() Calling request_parse_body() on a multipart request whose body was already parsed (at startup, or by a previous call) replaced SG(rfc1867_uploaded_files). The files from the first parse were no longer accepted by is_uploaded_file() and move_uploaded_file(), and were never deleted at the end of the request. --- .../multipart_after_startup_parse.phpt | 24 +++++++++++++++++ .../request_parse_body/multipart_twice.phpt | 27 +++++++++++++++++++ main/rfc1867.c | 9 ++++--- 3 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 ext/standard/tests/http/request_parse_body/multipart_after_startup_parse.phpt create mode 100644 ext/standard/tests/http/request_parse_body/multipart_twice.phpt diff --git a/ext/standard/tests/http/request_parse_body/multipart_after_startup_parse.phpt b/ext/standard/tests/http/request_parse_body/multipart_after_startup_parse.phpt new file mode 100644 index 000000000000..2193c51c629a --- /dev/null +++ b/ext/standard/tests/http/request_parse_body/multipart_after_startup_parse.phpt @@ -0,0 +1,24 @@ +--TEST-- +request_parse_body() keeps files uploaded during startup parsing +--POST_RAW-- +Content-Type: multipart/form-data; boundary=---------------------------84000087610663814162942123332 +-----------------------------84000087610663814162942123332 +Content-Disposition: form-data; name="file_name"; filename="original_file_name.txt" +Content-Type: text/plain + +file data +-----------------------------84000087610663814162942123332-- +--FILE-- + +--EXPECT-- +bool(true) +bool(true) diff --git a/ext/standard/tests/http/request_parse_body/multipart_twice.phpt b/ext/standard/tests/http/request_parse_body/multipart_twice.phpt new file mode 100644 index 000000000000..bf99aa3414ab --- /dev/null +++ b/ext/standard/tests/http/request_parse_body/multipart_twice.phpt @@ -0,0 +1,27 @@ +--TEST-- +request_parse_body() called twice keeps files uploaded by the first call +--ENV-- +REQUEST_METHOD=PUT +--POST_RAW-- +Content-Type: multipart/form-data; boundary=---------------------------84000087610663814162942123332 +-----------------------------84000087610663814162942123332 +Content-Disposition: form-data; name="file_name"; filename="original_file_name.txt" +Content-Type: text/plain + +file data +-----------------------------84000087610663814162942123332-- +--FILE-- + +--EXPECT-- +bool(true) +bool(true) diff --git a/main/rfc1867.c b/main/rfc1867.c index 39733e52c477..e7c228a79be4 100644 --- a/main/rfc1867.c +++ b/main/rfc1867.c @@ -694,7 +694,6 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) size_t array_len = 0; int64_t total_bytes = 0, max_file_size = 0; int skip_upload = 0, anonymous_index = 0; - HashTable *uploaded_files = NULL; multipart_buffer *mbuff; zval *array_ptr = (zval *) arg; bool throw_exceptions = SG(request_parse_body_context).throw_exceptions; @@ -792,9 +791,11 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* Initialize $_FILES[] */ zend_hash_init(&PG(rfc1867_protected_variables), 8, NULL, NULL, 0); - ALLOC_HASHTABLE(uploaded_files); - zend_hash_init(uploaded_files, 8, NULL, free_filename, 0); - SG(rfc1867_uploaded_files) = uploaded_files; + /* request_parse_body() may parse the body again, keep tracking earlier uploads */ + if (!SG(rfc1867_uploaded_files)) { + ALLOC_HASHTABLE(SG(rfc1867_uploaded_files)); + zend_hash_init(SG(rfc1867_uploaded_files), 8, NULL, free_filename, 0); + } if (Z_TYPE(PG(http_globals)[TRACK_VARS_FILES]) != IS_ARRAY) { /* php_auto_globals_create_files() might have already done that */ From dbd0a2d755602dbda9bc97454a4db4807f1093d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Thu, 12 Nov 2026 16:21:18 +0100 Subject: [PATCH 2/2] Check move_uploaded_file() in re-parse tests --- .../multipart_after_startup_parse.phpt | 20 ++++++++++++++++++- .../request_parse_body/multipart_twice.phpt | 20 ++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/ext/standard/tests/http/request_parse_body/multipart_after_startup_parse.phpt b/ext/standard/tests/http/request_parse_body/multipart_after_startup_parse.phpt index 2193c51c629a..0b3976ad6922 100644 --- a/ext/standard/tests/http/request_parse_body/multipart_after_startup_parse.phpt +++ b/ext/standard/tests/http/request_parse_body/multipart_after_startup_parse.phpt @@ -14,11 +14,29 @@ file data $tmp_name = $_FILES['file_name']['tmp_name']; var_dump(is_uploaded_file($tmp_name)); -request_parse_body(); +var_dump(request_parse_body()); var_dump(is_uploaded_file($tmp_name)); +$dst = __DIR__ . '/multipart_after_startup_parse.txt'; +var_dump(move_uploaded_file($tmp_name, $dst)); +var_dump(file_get_contents($dst)); + +?> +--CLEAN-- + --EXPECT-- bool(true) +array(2) { + [0]=> + array(0) { + } + [1]=> + array(0) { + } +} +bool(true) bool(true) +string(9) "file data" diff --git a/ext/standard/tests/http/request_parse_body/multipart_twice.phpt b/ext/standard/tests/http/request_parse_body/multipart_twice.phpt index bf99aa3414ab..27b6e58f94b8 100644 --- a/ext/standard/tests/http/request_parse_body/multipart_twice.phpt +++ b/ext/standard/tests/http/request_parse_body/multipart_twice.phpt @@ -17,11 +17,29 @@ file data $tmp_name = $files['file_name']['tmp_name']; var_dump(is_uploaded_file($tmp_name)); -request_parse_body(); +var_dump(request_parse_body()); var_dump(is_uploaded_file($tmp_name)); +$dst = __DIR__ . '/multipart_twice.txt'; +var_dump(move_uploaded_file($tmp_name, $dst)); +var_dump(file_get_contents($dst)); + +?> +--CLEAN-- + --EXPECT-- bool(true) +array(2) { + [0]=> + array(0) { + } + [1]=> + array(0) { + } +} +bool(true) bool(true) +string(9) "file data"