Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit aab7842

Browse files
committed
Fix GH-16998: UBSAN warning in rfc1867
The "else branch" of `next_line` can reset the `buf_begin` field to NULL, causing the next invocation to pass NULL to `memchr` with a 0 length. When UBSAN is enabled this causes an UBSAN abort. Real world impact is likely none because of the 0 length. To fix this, don't set the pointer to NULL, which means that the `memchr` will return NULL and since `self->bytes_in_buffer < self->bufsize` we return NULL and request more data through `fill_buffer`. That function will reset `buf_begin` and `bytes_in_buffer` so that the next invocation works fine. I chose this solution so we have an invariant that `buf_begin` is never NULL, which makes reasoning easier. An alternative solution is keeping the NULLing of `buf_begin` and add an extra check at the top of `next_line`, but I didn't like special casing this. Closes GH-17000.
1 parent 94fa2a4 commit aab7842

File tree

3 files changed

+53
-1
lines changed

3 files changed

+53
-1
lines changed

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ PHP NEWS
6060
. Fixed bug GH-15208 (Segfault with breakpoint map and phpdbg_clear()).
6161
(nielsdos)
6262

63+
- SAPI:
64+
. Fixed bug GH-16998 (UBSAN warning in rfc1867). (nielsdos)
65+
6366
- SimpleXML:
6467
. Fixed bug GH-16808 (Segmentation fault in RecursiveIteratorIterator
6568
->current() with a xml element input). (nielsdos)

‎main/rfc1867.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,8 @@ static char *next_line(multipart_buffer *self)
341341
}
342342
/* return entire buffer as a partial line */
343343
line[self->bufsize] = 0;
344-
self->buf_begin = ptr;
345344
self->bytes_in_buffer = 0;
345+
/* Let fill_buffer() handle the reset of self->buf_begin */
346346
}
347347

348348
return line;

‎tests/basic/gh16998.phpt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
GH-16998 (UBSAN warning in rfc1867)
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+
const FILLUNIT = 5 * 1024;
12+
$cmd = [
13+
getenv('TEST_PHP_CGI_EXECUTABLE'),
14+
'-C',
15+
'-n',
16+
__DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
17+
];
18+
$boundary = str_repeat('A', FILLUNIT);
19+
$body = ""
20+
. "--$boundary\r\n"
21+
. "Content-Disposition: form-data; name=\"koko\"\r\n"
22+
. "\r\n"
23+
. "BBB\r\n--" . substr($boundary, 0, -1) . "CCC\r\n"
24+
. "--$boundary--\r\n"
25+
;
26+
$env = array_merge($_ENV, [
27+
'REDIRECT_STATUS' => '1',
28+
'CONTENT_TYPE' => "multipart/form-data; boundary=",
29+
'CONTENT_LENGTH' => strlen($body),
30+
'REQUEST_METHOD' => 'POST',
31+
'SCRIPT_FILENAME' => __DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
32+
]);
33+
$spec = [
34+
0 => ['pipe', 'r'],
35+
1 => STDOUT,
36+
2 => STDOUT,
37+
];
38+
$pipes = [];
39+
$handle = proc_open($cmd, $spec, $pipes, getcwd(), $env);
40+
fwrite($pipes[0], $body);
41+
proc_close($handle);
42+
?>
43+
--EXPECTF--
44+
X-Powered-By: PHP/%s
45+
Content-type: text/html; charset=UTF-8
46+
47+
Hello world
48+
array(0) {
49+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /