File php7-CVE-2023-0662.patch of Package php7
From 716de0cff539f46294ef70fe75d548cd66766370 Mon Sep 17 00:00:00 2001
From: Jakub Zelenka <bukka@php.net>
Date: Thu, 19 Jan 2023 14:31:25 +0000
Subject: [PATCH] Introduce max_multipart_body_parts INI
This fixes GHSA-54hq-v5wp-fqgv DOS vulnerabality by limitting number of
parsed multipart body parts as currently all parts were always parsed.
---
main/main.c | 1 +
main/rfc1867.c | 11 ++
...-54hq-v5wp-fqgv-max-body-parts-custom.phpt | 53 +++++++++
...54hq-v5wp-fqgv-max-body-parts-default.phpt | 54 +++++++++
.../ghsa-54hq-v5wp-fqgv-max-file-uploads.phpt | 52 +++++++++
sapi/fpm/tests/tester.inc | 106 +++++++++++++++---
6 files changed, 262 insertions(+), 15 deletions(-)
create mode 100644 sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-body-parts-custom.phpt
create mode 100644 sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-body-parts-default.phpt
create mode 100644 sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-file-uploads.phpt
diff --git a/main/main.c b/main/main.c
index 40684f32dc14..c58ea58bf5ac 100644
--- a/main/main.c
+++ b/main/main.c
@@ -751,6 +751,7 @@ PHP_INI_BEGIN()
PHP_INI_ENTRY("disable_functions", "", PHP_INI_SYSTEM, NULL)
PHP_INI_ENTRY("disable_classes", "", PHP_INI_SYSTEM, NULL)
PHP_INI_ENTRY("max_file_uploads", "20", PHP_INI_SYSTEM|PHP_INI_PERDIR, NULL)
+ PHP_INI_ENTRY("max_multipart_body_parts", "-1", PHP_INI_SYSTEM|PHP_INI_PERDIR, NULL)
STD_PHP_INI_BOOLEAN("allow_url_fopen", "1", PHP_INI_SYSTEM, OnUpdateBool, allow_url_fopen, php_core_globals, core_globals)
STD_PHP_INI_BOOLEAN("allow_url_include", "0", PHP_INI_SYSTEM, OnUpdateBool, allow_url_include, php_core_globals, core_globals)
diff --git a/main/rfc1867.c b/main/rfc1867.c
index b43cfae5a1e2..3086e8da3dbe 100644
--- a/main/rfc1867.c
+++ b/main/rfc1867.c
@@ -687,6 +687,7 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
void *event_extra_data = NULL;
unsigned int llen = 0;
int upload_cnt = INI_INT("max_file_uploads");
+ int body_parts_cnt = INI_INT("max_multipart_body_parts");
const zend_encoding *internal_encoding = zend_multibyte_get_internal_encoding();
php_rfc1867_getword_t getword;
php_rfc1867_getword_conf_t getword_conf;
@@ -708,6 +709,11 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
return;
}
+ if (body_parts_cnt < 0) {
+ body_parts_cnt = PG(max_input_vars) + upload_cnt;
+ }
+ int body_parts_limit = body_parts_cnt;
+
/* Get the boundary */
boundary = strstr(content_type_dup, "boundary");
if (!boundary) {
@@ -792,6 +798,11 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
char *pair = NULL;
int end = 0;
+ if (--body_parts_cnt < 0) {
+ php_error_docref(NULL, E_WARNING, "Multipart body parts limit exceeded %d. To increase the limit change max_multipart_body_parts in php.ini.", body_parts_limit);
+ goto fileupload_done;
+ }
+
while (isspace(*cd)) {
++cd;
}
diff --git a/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-body-parts-custom.phpt b/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-body-parts-custom.phpt
new file mode 100644
index 000000000000..d2239ac3c410
--- /dev/null
+++ b/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-body-parts-custom.phpt
@@ -0,0 +1,53 @@
+--TEST--
+FPM: GHSA-54hq-v5wp-fqgv - max_multipart_body_parts ini custom value
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+php_admin_value[html_errors] = false
+php_admin_value[max_input_vars] = 20
+php_admin_value[max_file_uploads] = 5
+php_admin_value[max_multipart_body_parts] = 10
+php_flag[display_errors] = On
+EOT;
+
+$code = <<<EOT
+<?php
+var_dump(count(\$_POST));
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+echo $tester
+ ->request(stdin: [
+ 'parts' => [
+ 'count' => 30,
+ ]
+ ])
+ ->getBody();
+$tester->terminate();
+$tester->close();
+
+?>
+--EXPECT--
+Warning: Unknown: Multipart body parts limit exceeded 10. To increase the limit change max_multipart_body_parts in php.ini. in Unknown on line 0
+int(10)
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git a/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-body-parts-default.phpt b/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-body-parts-default.phpt
new file mode 100644
index 000000000000..42b5afbf9ee7
--- /dev/null
+++ b/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-body-parts-default.phpt
@@ -0,0 +1,54 @@
+--TEST--
+FPM: GHSA-54hq-v5wp-fqgv - max_multipart_body_parts ini default
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+php_admin_value[html_errors] = false
+php_admin_value[max_input_vars] = 20
+php_admin_value[max_file_uploads] = 5
+php_flag[display_errors] = On
+EOT;
+
+$code = <<<EOT
+<?php
+var_dump(count(\$_POST));
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+echo $tester
+ ->request(stdin: [
+ 'parts' => [
+ 'count' => 30,
+ ]
+ ])
+ ->getBody();
+$tester->terminate();
+$tester->close();
+
+?>
+--EXPECT--
+Warning: Unknown: Input variables exceeded 20. To increase the limit change max_input_vars in php.ini. in Unknown on line 0
+
+Warning: Unknown: Multipart body parts limit exceeded 25. To increase the limit change max_multipart_body_parts in php.ini. in Unknown on line 0
+int(20)
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git a/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-file-uploads.phpt b/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-file-uploads.phpt
new file mode 100644
index 000000000000..da81174c7280
--- /dev/null
+++ b/sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-file-uploads.phpt
@@ -0,0 +1,52 @@
+--TEST--
+FPM: GHSA-54hq-v5wp-fqgv - exceeding max_file_uploads
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+php_admin_value[html_errors] = false
+php_admin_value[max_file_uploads] = 5
+php_flag[display_errors] = On
+EOT;
+
+$code = <<<EOT
+<?php
+var_dump(count(\$_FILES));
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+echo $tester
+ ->request(stdin: [
+ 'parts' => [
+ 'count' => 10,
+ 'param' => 'filename'
+ ]
+ ])
+ ->getBody();
+$tester->terminate();
+$tester->close();
+
+?>
+--EXPECT--
+Warning: Maximum number of allowable file uploads has been exceeded in Unknown on line 0
+int(5)
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc
index 6197cdba53f5..e51aa0f69143 100644
--- a/sapi/fpm/tests/tester.inc
+++ b/sapi/fpm/tests/tester.inc
@@ -531,12 +531,16 @@ class Tester
* @param string|null $successMessage
* @param string|null $errorMessage
* @param bool $connKeepAlive
+ * @param string|null $scriptFilename
+ * @param string|null $stdin
* @return array
*/
private function getRequestParams(
string $query = '',
array $headers = [],
- string $uri = null
+ string $uri = null,
+ string $scriptFilename = null,
+ ?string $stdin = null
) {
if (is_null($uri)) {
$uri = $this->makeSourceFile();
@@ -545,8 +549,8 @@ class Tester
$params = array_merge(
[
'GATEWAY_INTERFACE' => 'FastCGI/1.0',
- 'REQUEST_METHOD' => 'GET',
- 'SCRIPT_FILENAME' => $uri,
+ 'REQUEST_METHOD' => is_null($stdin) ? 'GET' : 'POST',
+ 'SCRIPT_FILENAME' => $scriptFilename ?: $uri,
'SCRIPT_NAME' => $uri,
'QUERY_STRING' => $query,
'REQUEST_URI' => $uri . ($query ? '?'.$query : ""),
@@ -560,7 +564,7 @@ class Tester
'SERVER_PROTOCOL' => 'HTTP/1.1',
'DOCUMENT_ROOT' => __DIR__,
'CONTENT_TYPE' => '',
- 'CONTENT_LENGTH' => 0
+ 'CONTENT_LENGTH' => strlen($stdin ?? "") // Default to 0
],
$headers
);
@@ -571,6 +575,69 @@ class Tester
}
/**
+ * Parse stdin and generate data for multipart config.
+ *
+ * @param array $stdin
+ * @param array $headers
+ *
+ * @return void
+ * @throws \Exception
+ */
+ private function parseStdin(array $stdin, array &$headers)
+ {
+ $parts = $stdin['parts'] ?? null;
+ if (empty($parts)) {
+ throw new \Exception('The stdin array needs to contain parts');
+ }
+ $boundary = $stdin['boundary'] ?? 'AaB03x';
+ if ( ! isset($headers['CONTENT_TYPE'])) {
+ $headers['CONTENT_TYPE'] = 'multipart/form-data; boundary=' . $boundary;
+ }
+ $count = $parts['count'] ?? null;
+ if ( ! is_null($count)) {
+ $dispositionType = $parts['disposition'] ?? 'form-data';
+ $dispositionParam = $parts['param'] ?? 'name';
+ $namePrefix = $parts['prefix'] ?? 'f';
+ $nameSuffix = $parts['suffix'] ?? '';
+ $value = $parts['value'] ?? 'test';
+ $parts = [];
+ for ($i = 0; $i < $count; $i++) {
+ $parts[] = [
+ 'disposition' => $dispositionType,
+ 'param' => $dispositionParam,
+ 'name' => "$namePrefix$i$nameSuffix",
+ 'value' => $value
+ ];
+ }
+ }
+ $out = '';
+ $nl = "\r\n";
+ foreach ($parts as $part) {
+ if (!is_array($part)) {
+ $part = ['name' => $part];
+ } elseif ( ! isset($part['name'])) {
+ throw new \Exception('Each part has to have a name');
+ }
+ $name = $part['name'];
+ $dispositionType = $part['disposition'] ?? 'form-data';
+ $dispositionParam = $part['param'] ?? 'name';
+ $value = $part['value'] ?? 'test';
+ $partHeaders = $part['headers'] ?? [];
+
+ $out .= "--$boundary$nl";
+ $out .= "Content-disposition: $dispositionType; $dispositionParam=\"$name\"$nl";
+ foreach ($partHeaders as $headerName => $headerValue) {
+ $out .= "$headerName: $headerValue$nl";
+ }
+ $out .= $nl;
+ $out .= "$value$nl";
+ }
+ $out .= "--$boundary--$nl";
+
+ return $out;
+ }
+
+ /**
* Execute request.
*
* @param string $query
@@ -580,7 +647,10 @@ class Tester
* @param string|null $successMessage
* @param string|null $errorMessage
* @param bool $connKeepAlive
+ * @param string|null $scriptFilename = null
+ * @param string|array|null $stdin = null
* @return Response
+ * @throws \Exception
*/
public function request(
string $query = '',
@@ -589,17 +659,24 @@ class Tester
string $address = null,
string $successMessage = null,
string $errorMessage = null,
- bool $connKeepAlive = false
+ bool $connKeepAlive = false,
+ string $scriptFilename = null,
+ string|array $stdin = null
) {
if ($this->hasError()) {
return new Response(null, true);
}
- $params = $this->getRequestParams($query, $headers, $uri);
+ if (is_array($stdin)) {
+ $stdin = $this->parseStdin($stdin, $headers);
+ }
+
+ $params = $this->getRequestParams($query, $headers, $uri, $scriptFilename, $stdin);
+ $this->trace('Request params', $params);
try {
$this->response = new Response(
- $this->getClient($address, $connKeepAlive)->request_data($params, false)
+ $this->getClient($address, $connKeepAlive)->request_data($params, $stdin)
);
$this->message($successMessage);
} catch (\Exception $exception) {
