Fix GHSA-9pqp-7h25-4f32

multipart/form-data boundaries larger than the read buffer result in erroneous
parsing, which violates data integrity.

Limit boundary size, as allowed by RFC 1521:

    Encapsulation boundaries [...] must be no longer than 70 characters, not
    counting the two leading hyphens.

We correctly parse payloads with boundaries of length up to
FILLUNIT-strlen("\r\n--") bytes, so allow this for BC.
This commit is contained in:
Arnaud Le Blanc 2024-09-09 15:22:07 +02:00 committed by Jakub Zelenka
parent a87ccc7ca2
commit d65a1e6f91
No known key found for this signature in database
GPG key ID: 1C0779DC5C0A9DE4
3 changed files with 110 additions and 0 deletions

View file

@ -751,6 +751,13 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
boundary_len = boundary_end-boundary;
}
/* Boundaries larger than FILLUNIT-strlen("\r\n--") characters lead to
* erroneous parsing */
if (boundary_len > FILLUNIT-strlen("\r\n--")) {
sapi_module.sapi_error(E_WARNING, "Boundary too large in multipart/form-data POST data");
return;
}
/* Initialize the buffer */
if (!(mbuff = multipart_buffer_new(boundary, boundary_len))) {
sapi_module.sapi_error(E_WARNING, "Unable to initialize the input buffer");

View file

@ -0,0 +1,3 @@
<?php
print "Hello world\n";
var_dump($_POST);

View file

@ -0,0 +1,100 @@
--TEST--
GHSA-9pqp-7h25-4f32
--SKIPIF--
<?php
if (!getenv('TEST_PHP_CGI_EXECUTABLE')) {
die("skip php-cgi not available");
}
?>
--FILE--
<?php
const FILLUNIT = 5 * 1024;
function test($boundaryLen) {
printf("Boundary len: %d\n", $boundaryLen);
$cmd = [
getenv('TEST_PHP_CGI_EXECUTABLE'),
'-C',
'-n',
__DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
];
$boundary = str_repeat('A', $boundaryLen);
$body = ""
. "--$boundary\r\n"
. "Content-Disposition: form-data; name=\"koko\"\r\n"
. "\r\n"
. "BBB\r\n--" . substr($boundary, 0, -1) . "CCC\r\n"
. "--$boundary--\r\n"
;
$env = array_merge($_ENV, [
'REDIRECT_STATUS' => '1',
'CONTENT_TYPE' => "multipart/form-data; boundary=$boundary",
'CONTENT_LENGTH' => strlen($body),
'REQUEST_METHOD' => 'POST',
'SCRIPT_FILENAME' => __DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
]);
$spec = [
0 => ['pipe', 'r'],
1 => STDOUT,
2 => STDOUT,
];
$pipes = [];
print "Starting...\n";
$handle = proc_open($cmd, $spec, $pipes, getcwd(), $env);
fwrite($pipes[0], $body);
$status = proc_close($handle);
print "\n";
}
for ($offset = -1; $offset <= 1; $offset++) {
test(FILLUNIT - strlen("\r\n--") + $offset);
}
?>
--EXPECTF--
Boundary len: 5115
Starting...
X-Powered-By: %s
Content-type: text/html; charset=UTF-8
Hello world
array(1) {
["koko"]=>
string(5124) "BBB
--AAA%sCCC"
}
Boundary len: 5116
Starting...
X-Powered-By: %s
Content-type: text/html; charset=UTF-8
Hello world
array(1) {
["koko"]=>
string(5125) "BBB
--AAA%sCCC"
}
Boundary len: 5117
Starting...
X-Powered-By: %s
Content-type: text/html; charset=UTF-8
<br />
<b>Warning</b>: Boundary too large in multipart/form-data POST data in <b>Unknown</b> on line <b>0</b><br />
Hello world
array(0) {
}