Skip to content

Commit cc487bc

Browse files
arnaud-lbremicollet
authored andcommitted
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. (cherry picked from commit 19b49258d0c5a61398d395d8afde1123e8d161e0) (cherry picked from commit 2b0daf4) (cherry picked from commit a24ac17) (cherry picked from commit 08f0adf) (cherry picked from commit 5731a40) (cherry picked from commit c9e67e9)
1 parent e925cc5 commit cc487bc

File tree

3 files changed

+110
-0
lines changed

3 files changed

+110
-0
lines changed

main/rfc1867.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,13 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) /* {{{ */
767767
boundary_len = boundary_end-boundary;
768768
}
769769

770+
/* Boundaries larger than FILLUNIT-strlen("\r\n--") characters lead to
771+
* erroneous parsing */
772+
if (boundary_len > FILLUNIT-strlen("\r\n--")) {
773+
sapi_module.sapi_error(E_WARNING, "Boundary too large in multipart/form-data POST data");
774+
return;
775+
}
776+
770777
/* Initialize the buffer */
771778
if (!(mbuff = multipart_buffer_new(boundary, boundary_len))) {
772779
sapi_module.sapi_error(E_WARNING, "Unable to initialize the input buffer");

tests/basic/GHSA-9pqp-7h25-4f32.inc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?php
2+
print "Hello world\n";
3+
var_dump($_POST);

tests/basic/GHSA-9pqp-7h25-4f32.phpt

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
--TEST--
2+
GHSA-9pqp-7h25-4f32
3+
--SKIPIF--
4+
<?php
5+
if (!getenv('TEST_PHP_CGI_EXECUTABLE')) {
6+
die("skip php-cgi not available");
7+
}
8+
?>
9+
--FILE--
10+
<?php
11+
12+
const FILLUNIT = 5 * 1024;
13+
14+
function test($boundaryLen) {
15+
printf("Boundary len: %d\n", $boundaryLen);
16+
17+
$cmd = [
18+
getenv('TEST_PHP_CGI_EXECUTABLE'),
19+
'-C',
20+
'-n',
21+
__DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
22+
];
23+
24+
$boundary = str_repeat('A', $boundaryLen);
25+
$body = ""
26+
. "--$boundary\r\n"
27+
. "Content-Disposition: form-data; name=\"koko\"\r\n"
28+
. "\r\n"
29+
. "BBB\r\n--" . substr($boundary, 0, -1) . "CCC\r\n"
30+
. "--$boundary--\r\n"
31+
;
32+
33+
$env = array_merge($_ENV, [
34+
'REDIRECT_STATUS' => '1',
35+
'CONTENT_TYPE' => "multipart/form-data; boundary=$boundary",
36+
'CONTENT_LENGTH' => strlen($body),
37+
'REQUEST_METHOD' => 'POST',
38+
'SCRIPT_FILENAME' => __DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
39+
]);
40+
41+
$spec = [
42+
0 => ['pipe', 'r'],
43+
1 => STDOUT,
44+
2 => STDOUT,
45+
];
46+
47+
$pipes = [];
48+
49+
print "Starting...\n";
50+
51+
$handle = proc_open($cmd, $spec, $pipes, getcwd(), $env);
52+
53+
fwrite($pipes[0], $body);
54+
55+
$status = proc_close($handle);
56+
57+
print "\n";
58+
}
59+
60+
for ($offset = -1; $offset <= 1; $offset++) {
61+
test(FILLUNIT - strlen("\r\n--") + $offset);
62+
}
63+
64+
?>
65+
--EXPECTF--
66+
Boundary len: 5115
67+
Starting...
68+
X-Powered-By: %s
69+
Content-type: text/html; charset=UTF-8
70+
71+
Hello world
72+
array(1) {
73+
["koko"]=>
74+
string(5124) "BBB
75+
--AAA%sCCC"
76+
}
77+
78+
Boundary len: 5116
79+
Starting...
80+
X-Powered-By: %s
81+
Content-type: text/html; charset=UTF-8
82+
83+
Hello world
84+
array(1) {
85+
["koko"]=>
86+
string(5125) "BBB
87+
--AAA%sCCC"
88+
}
89+
90+
Boundary len: 5117
91+
Starting...
92+
X-Powered-By: %s
93+
Content-type: text/html; charset=UTF-8
94+
95+
<br />
96+
<b>Warning</b>: Boundary too large in multipart/form-data POST data in <b>Unknown</b> on line <b>0</b><br />
97+
Hello world
98+
array(0) {
99+
}
100+

0 commit comments

Comments
 (0)