Skip to content

Commit b750803

Browse files
committed
ext/standard: fix out-of-bounds access in the ftp:// directory stream
php_ftp_dirstream_read() sized the basename copy with MIN(sizeof(ent->d_name), ZSTR_LEN(basename) - 1) and wrote the terminator at [tmp_len - 1]. An empty listing line leaves a basename of length 1, so tmp_len is 0 and the NUL lands one byte before d_name; a slash-only line leaves length 0, so the subtraction underflows to SIZE_MAX and memcpy() reads 4096 bytes past the interned empty string. The same off-by-one truncated entry names that do not end in CRLF. Closes GH-22768
1 parent 1c4bf04 commit b750803

5 files changed

Lines changed: 38 additions & 5 deletions

File tree

ext/ftp/tests/server.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ if ($pid) {
290290
}
291291

292292
if (empty($m[1]) || $m[1] !== 'emptydir') {
293-
fputs($fs, "file1\r\nfile1\r\nfile\nb0rk\r\n");
293+
fputs($fs, $nlst_data ?? "file1\r\nfile1\r\nfile\nb0rk\r\n");
294294
}
295295

296296
fputs($s, "226 Closing data Connection.\r\n");

ext/standard/ftp_fopen_wrapper.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -625,9 +625,9 @@ static ssize_t php_ftp_dirstream_read(php_stream *stream, char *buf, size_t coun
625625

626626
basename = php_basename(ent->d_name, tmp_len, NULL, 0);
627627

628-
tmp_len = MIN(sizeof(ent->d_name), ZSTR_LEN(basename) - 1);
628+
tmp_len = MIN(sizeof(ent->d_name) - 1, ZSTR_LEN(basename));
629629
memcpy(ent->d_name, ZSTR_VAL(basename), tmp_len);
630-
ent->d_name[tmp_len - 1] = '\0';
630+
ent->d_name[tmp_len] = '\0';
631631
zend_string_release_ex(basename, 0);
632632
ent->d_type = DT_UNKNOWN;
633633

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
opendir() with 'ftp://' stream must not go out of bounds on an empty or slash-only listing line
3+
--SKIPIF--
4+
<?php
5+
if (array_search('ftp',stream_get_wrappers()) === FALSE) die("skip ftp wrapper not available.");
6+
if (!function_exists('pcntl_fork')) die("skip pcntl_fork() not available.");
7+
?>
8+
--FILE--
9+
<?php
10+
11+
/* An empty line makes php_basename() return "\n" and a slash-only final line
12+
* makes it return "", the two lengths the buffer math underflowed on. */
13+
$nlst_data = "file1\r\n\nb0rk\r\n/";
14+
15+
require __DIR__ . "/../../../ftp/tests/server.inc";
16+
17+
$path="ftp://localhost:" . $port."/";
18+
19+
$ds=opendir($path);
20+
var_dump($ds);
21+
22+
while (($fn=readdir($ds)) !== false) {
23+
var_dump($fn);
24+
}
25+
26+
closedir($ds);
27+
?>
28+
--EXPECTF--
29+
resource(%d) of type (stream)
30+
string(5) "file1"
31+
string(0) ""
32+
string(4) "b0rk"
33+
string(0) ""

ext/standard/tests/streams/opendir-002.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,5 @@ closedir($ds);
2525
resource(%d) of type (stream)
2626
string(5) "file1"
2727
string(5) "file1"
28-
string(3) "fil"
28+
string(4) "file"
2929
string(4) "b0rk"

ext/standard/tests/streams/opendir-004.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,5 @@ while ($fn=readdir($ds)) {
2727
resource(%d) of type (stream)
2828
string(5) "file1"
2929
string(5) "file1"
30-
string(3) "fil"
30+
string(4) "file"
3131
string(4) "b0rk"

0 commit comments

Comments
 (0)