File php7-CVE-2023-0662.patch of Package php7

File php7-CVE-2023-0662.patch of Package php7 (Revision 66dbe4b637d5c3ff94215f64249a08dc)

Currently displaying revision 66dbe4b637d5c3ff94215f64249a08dc , Show latest

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) {