Skip to content

Commit ba22647

Browse files
committed
ext/ftp: fix out-of-bounds read in ftp_get() ASCII CRLF translation
In ASCII mode ftp_get() scans each received block for '\r' and peeks at the next byte to collapse a CRLF pair to '\n'. When the '\r' is the last byte of a full FTP_BUFSIZE block, the *(s + 1) lookahead reads one byte past the data buffer; a server placing '\r' at offset 4095 of a 4096-byte read triggers it. Bound the lookahead to the received data, matching the guard ftp_readline() already uses. ftp_nb_continue_read() carries the trailing '\r' across reads via ftp->lastch and is unaffected. Closes GH-22328
1 parent 0c52780 commit ba22647

3 files changed

Lines changed: 33 additions & 1 deletion

File tree

ext/ftp/ftp.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,7 @@ ftp_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_t pat
946946
#else
947947
while (e > ptr && (s = memchr(ptr, '\r', (e - ptr)))) {
948948
php_stream_write(outstream, ptr, (s - ptr));
949-
if (*(s + 1) == '\n') {
949+
if (s + 1 < e && *(s + 1) == '\n') {
950950
s++;
951951
php_stream_putc(outstream, '\n');
952952
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
ftp_get() ASCII mode: CRLF straddling the FTP_BUFSIZE read boundary
3+
--EXTENSIONS--
4+
ftp
5+
pcntl
6+
--FILE--
7+
<?php
8+
require 'server.inc';
9+
10+
$ftp = ftp_connect('127.0.0.1', $port);
11+
ftp_login($ftp, 'user', 'pass');
12+
$ftp or die("Couldn't connect to the server");
13+
14+
$local = __DIR__ . "/crlf_boundary_out.txt";
15+
16+
var_dump(ftp_get($ftp, $local, 'crlf_boundary', FTP_ASCII));
17+
var_dump(file_get_contents($local) === str_repeat("A", 4095) . "\n" . str_repeat("B", 10));
18+
?>
19+
--CLEAN--
20+
<?php
21+
@unlink(__DIR__ . "/crlf_boundary_out.txt");
22+
?>
23+
--EXPECT--
24+
bool(true)
25+
bool(true)

ext/ftp/tests/server.inc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,13 @@ if ($pid) {
391391
// Just a side channel for getting the received file size.
392392
fputs($s, "425 Can't open data connection (".$GLOBALS['rest_pos'].").\r\n");
393393
break;
394+
case "crlf_boundary":
395+
// A CRLF whose CR lands on the final byte of the first
396+
// FTP_BUFSIZE (4096) read, so the LF arrives in the next read.
397+
fputs($s, "150 File status okay; about to open data connection.\r\n");
398+
fputs($fs, str_repeat("A", 4095) . "\r\n" . str_repeat("B", 10));
399+
fputs($s, "226 Closing data Connection.\r\n");
400+
break;
394401

395402
default:
396403
fputs($s, "550 {$matches[1]}: No such file or directory \r\n");

0 commit comments

Comments
 (0)