Commit 7774891f161 for php
commit 7774891f161faab183e6c83572a49e37cec8fed8
Author: Alejandro Pacheco <alepacheco99@gmail.com>
Date: Thu Oct 15 04:45:51 2026 +0200
Fix out-of-bounds reads parsing ftp:// wrapper server replies
The ftp:// wrapper parses replies in a 512-byte buffer at fixed offsets
and sizes that are not tied to the line actually received:
- get_ftp_result() returns the code of the last line read when the
stream ends, even if it is not a complete "ddd " status line, while
the callers parse the reply text from tmp_line + 4.
- php_fopen_do_pasv() copies a fixed ip_size bytes from the dotted
address inside the buffer, reading past its end when the address is
near the end of the line, and silently truncating an over-long one.
- The MDTM parsing in php_stream_ftp_url_stat() skips non-digits past
the terminating NUL, and its bound check allows sscanf() to start one
past the end of the buffer.
Only return a code from get_ftp_result() for a complete status line,
copy exactly the parsed address and reject one that does not fit, and
stop the MDTM scan at the end of the line.
Closes GH-24340.
diff --git a/NEWS b/NEWS
index 6a131ab3ded..d79257caf33 100644
--- a/NEWS
+++ b/NEWS
@@ -343,6 +343,8 @@ PHP NEWS
parsing fails after Transfer-Encoding). (bercedev)
. Fixed bug GH-24509 (HTTP wrapper leaks Location buffer on empty
Location header). (bercedev)
+ . Fix out-of-bounds reads parsing ftp:// wrapper server replies.
+ (Alejandro Pacheco)
- Tidy:
. Fixed tidy_repair_string(), tidy_repair_file(), tidy::repairString() and
diff --git a/ext/ftp/tests/server.inc b/ext/ftp/tests/server.inc
index 2d9697e4d35..00a64d76e6f 100644
--- a/ext/ftp/tests/server.inc
+++ b/ext/ftp/tests/server.inc
@@ -412,6 +412,10 @@ if ($pid) {
fclose($fs);
+ }elseif (preg_match('/^PASV/', $buf, $matches) && isset($pasv_reply_at_buffer_end)) {
+ // The address ends at the 510th byte and the port is cut off.
+ fputs($s, "227 " . str_repeat("x", 496) . "127,0,0,1,4,210)\r\n");
+
}elseif (preg_match('/^PASV/', $buf, $matches)) {
$pasv=true;
$host = "127.0.0.1";
@@ -542,6 +546,13 @@ if ($pid) {
fclose($fs);
}elseif (preg_match('/^SIZE \/bug73457/', $buf)) {
fputs($s, "213 10\r\n");
+ }elseif (preg_match('/^SIZE \/pasvbufferend/', $buf)) {
+ fputs($s, "213 10\r\n");
+ }elseif (preg_match('/^SIZE \/mdtmnodigits/', $buf)) {
+ // A longer line with digits, read into the same buffer before MDTM.
+ fputs($s, "213 10 bytes (19991005213102)\r\n");
+ }elseif (preg_match('/^MDTM \/mdtmnodigits/', $buf)) {
+ fputs($s, "213 unknown\r\n");
}elseif (preg_match("/^SITE/", $buf)) {
fputs($s, "500 Syntax error, command unrecognized.\r\n");
}else {
diff --git a/ext/standard/ftp_fopen_wrapper.c b/ext/standard/ftp_fopen_wrapper.c
index fd7ecec02f1..a4eb57db9e5 100644
--- a/ext/standard/ftp_fopen_wrapper.c
+++ b/ext/standard/ftp_fopen_wrapper.c
@@ -74,13 +74,23 @@ typedef struct _php_ftp_dirstream_data {
} php_ftp_dirstream_data;
/* {{{ get_ftp_result */
+/* Reads reply lines into buffer until a complete status line ("ddd ") and
+ * returns its code, or 0 if the stream ends before one.
+ * On return the buffer always holds a NUL-terminated string, which the
+ * callers rely on when they parse or print the reply text: it is emptied
+ * first in case nothing can be read, and php_stream_gets() terminates every
+ * line it reads. */
static inline int get_ftp_result(php_stream *stream, char *buffer, size_t buffer_size)
{
buffer[0] = '\0'; /* in case read fails to read anything */
- while (php_stream_gets(stream, buffer, buffer_size-1) &&
- !(isdigit((unsigned char)buffer[0]) && isdigit((unsigned char)buffer[1]) &&
- isdigit((unsigned char)buffer[2]) && buffer[3] == ' '));
- return strtol(buffer, NULL, 10);
+ while (php_stream_gets(stream, buffer, buffer_size-1)) {
+ if (isdigit((unsigned char)buffer[0]) && isdigit((unsigned char)buffer[1]) &&
+ isdigit((unsigned char)buffer[2]) && buffer[3] == ' ') {
+ return strtol(buffer, NULL, 10);
+ }
+ }
+ /* no complete status line, callers parse the reply from buffer + 4 */
+ return 0;
}
/* }}} */
@@ -352,8 +362,10 @@ static unsigned short php_fopen_do_pasv(php_stream *stream, char *ip, size_t ip_
tpath++;
}
tpath[-1] = '\0';
- memcpy(ip, hoststart, ip_size);
- ip[ip_size-1] = '\0';
+ if ((size_t)(tpath - hoststart) > ip_size) {
+ return 0;
+ }
+ memcpy(ip, hoststart, tpath - hoststart);
hoststart = ip;
/* pull out the MSB of the port */
@@ -825,11 +837,11 @@ static int php_stream_ftp_url_stat(php_stream_wrapper *wrapper, const char *url,
struct tm tm, tmbuf, *gmt;
time_t stamp;
- while ((size_t)(p - tmp_line) < sizeof(tmp_line) && !isdigit((unsigned char)*p)) {
+ while (*p && !isdigit((unsigned char)*p)) {
p++;
}
- if ((size_t)(p - tmp_line) > sizeof(tmp_line)) {
+ if (!*p) {
goto mdtm_error;
}
diff --git a/ext/standard/tests/streams/ftp_fopen_do_pasv_reply_bounds.phpt b/ext/standard/tests/streams/ftp_fopen_do_pasv_reply_bounds.phpt
new file mode 100644
index 00000000000..2cdadd41541
--- /dev/null
+++ b/ext/standard/tests/streams/ftp_fopen_do_pasv_reply_bounds.phpt
@@ -0,0 +1,20 @@
+--TEST--
+ftp:// wrapper rejects a PASV reply whose address ends at the reply buffer limit
+--SKIPIF--
+<?php
+if (array_search('ftp',stream_get_wrappers()) === FALSE) die("skip ftp wrapper not available.");
+if (!function_exists('pcntl_fork')) die("skip pcntl_fork() not available.");
+?>
+--FILE--
+<?php
+
+$pasv_reply_at_buffer_end = true;
+require __DIR__ . "/../../../ftp/tests/server.inc";
+
+$path = "ftp://localhost:" . $port . "/pasvbufferend";
+
+var_dump(fopen($path, "r"));
+?>
+--EXPECTF--
+Warning: fopen(%s): Failed to open stream: FTP server reports %a in %s on line %d
+bool(false)
diff --git a/ext/standard/tests/streams/ftp_url_stat_mdtm_no_digits.phpt b/ext/standard/tests/streams/ftp_url_stat_mdtm_no_digits.phpt
new file mode 100644
index 00000000000..033c8f016a0
--- /dev/null
+++ b/ext/standard/tests/streams/ftp_url_stat_mdtm_no_digits.phpt
@@ -0,0 +1,20 @@
+--TEST--
+stat() on ftp:// ignores an MDTM reply without digits
+--SKIPIF--
+<?php
+if (array_search('ftp',stream_get_wrappers()) === FALSE) die("skip ftp wrapper not available.");
+if (!function_exists('pcntl_fork')) die("skip pcntl_fork() not available.");
+?>
+--FILE--
+<?php
+
+require __DIR__ . "/../../../ftp/tests/server.inc";
+
+$path = "ftp://localhost:" . $port . "/mdtmnodigits";
+
+$stat = stat($path);
+var_dump($stat['size'], $stat['mtime']);
+?>
+--EXPECT--
+int(10)
+int(-1)