mirror of
https://github.com/php/php-src.git
synced 2025-08-16 05:58:45 +02:00
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.
This commit is contained in:
Jakub Zelenka
parent
830bdb582f
commit
94fce68f03
7 changed files with 262 additions and 15 deletions
4
NEWS
4
NEWS
|
|
@ -8,6 +8,10 @@ PHP NEWS
|
|||
. Fixed bug #81746 (1-byte array overrun in common path resolve code).
|
||||
(CVE-2023-0568). (Niels Dossche)
|
||||
|
||||
- FPM
|
||||
. Fixed bug GHSA-54hq-v5wp-fqgv (DOS vulnerability when parsing multipart
|
||||
request body). (CVE-2023-0662) (Jakub Zelenka)
|
||||
|
||||
02 Feb 2023, PHP 8.1.15
|
||||
|
||||
- Apache:
|
||||
|
|
|
|||
|
|
@ -746,6 +746,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)
|
||||
|
|
|
|||
|
|
@ -686,6 +686,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;
|
||||
|
|
@ -707,6 +708,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) {
|
||||
|
|
@ -791,6 +797,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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
?>
|
||||
|
|
@ -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();
|
||||
?>
|
||||
52
sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-file-uploads.phpt
Normal file
52
sapi/fpm/tests/ghsa-54hq-v5wp-fqgv-max-file-uploads.phpt
Normal file
|
|
@ -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();
|
||||
?>
|
||||
|
|
@ -574,6 +574,7 @@ class Tester
|
|||
* @param array $headers
|
||||
* @param string|null $uri
|
||||
* @param string|null $scriptFilename
|
||||
* @param string|null $stdin
|
||||
*
|
||||
* @return array
|
||||
*/
|
||||
|
|
@ -581,7 +582,8 @@ class Tester
|
|||
string $query = '',
|
||||
array $headers = [],
|
||||
string $uri = null,
|
||||
string $scriptFilename = null
|
||||
string $scriptFilename = null,
|
||||
?string $stdin = null
|
||||
): array {
|
||||
if (is_null($uri)) {
|
||||
$uri = $this->makeSourceFile();
|
||||
|
|
@ -590,7 +592,7 @@ class Tester
|
|||
$params = array_merge(
|
||||
[
|
||||
'GATEWAY_INTERFACE' => 'FastCGI/1.0',
|
||||
'REQUEST_METHOD' => 'GET',
|
||||
'REQUEST_METHOD' => is_null($stdin) ? 'GET' : 'POST',
|
||||
'SCRIPT_FILENAME' => $scriptFilename ?: $uri,
|
||||
'SCRIPT_NAME' => $uri,
|
||||
'QUERY_STRING' => $query,
|
||||
|
|
@ -605,7 +607,7 @@ class Tester
|
|||
'SERVER_PROTOCOL' => 'HTTP/1.1',
|
||||
'DOCUMENT_ROOT' => __DIR__,
|
||||
'CONTENT_TYPE' => '',
|
||||
'CONTENT_LENGTH' => 0
|
||||
'CONTENT_LENGTH' => strlen($stdin ?? "") // Default to 0
|
||||
],
|
||||
$headers
|
||||
);
|
||||
|
|
@ -615,21 +617,86 @@ 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
|
||||
* @param array $headers
|
||||
* @param string|null $uri
|
||||
* @param string|null $address
|
||||
* @param string|null $successMessage
|
||||
* @param string|null $errorMessagereadLimit
|
||||
* @param bool $connKeepAlive
|
||||
* @param string|null $scriptFilename = null
|
||||
* @param bool $expectError
|
||||
* @param int $readLimit
|
||||
* @param string $query
|
||||
* @param array $headers
|
||||
* @param string|null $uri
|
||||
* @param string|null $address
|
||||
* @param string|null $successMessage
|
||||
* @param string|null $errorMessage
|
||||
* @param bool $connKeepAlive
|
||||
* @param string|null $scriptFilename = null
|
||||
* @param string|array|null $stdin = null
|
||||
* @param bool $expectError
|
||||
* @param int $readLimit
|
||||
*
|
||||
* @return Response
|
||||
* @throws \Exception
|
||||
*/
|
||||
public function request(
|
||||
string $query = '',
|
||||
|
|
@ -640,6 +707,7 @@ class Tester
|
|||
string $errorMessage = null,
|
||||
bool $connKeepAlive = false,
|
||||
string $scriptFilename = null,
|
||||
string|array $stdin = null,
|
||||
bool $expectError = false,
|
||||
int $readLimit = -1,
|
||||
): Response {
|
||||
|
|
@ -647,12 +715,16 @@ class Tester
|
|||
return new Response(null, true);
|
||||
}
|
||||
|
||||
$params = $this->getRequestParams($query, $headers, $uri, $scriptFilename);
|
||||
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, $readLimit)
|
||||
$this->getClient($address, $connKeepAlive)->request_data($params, $stdin, $readLimit)
|
||||
);
|
||||
if ($expectError) {
|
||||
$this->error('Expected request error but the request was successful');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue