Skip to content

Commit

Permalink
Fix GH-10521: ftp_get/ftp_nb_get resumepos offset is maximum 10GB
Browse files Browse the repository at this point in the history
The char arrays were too small for a long on 64-bit systems, which
resulted in cutting off the string at the end with a NUL byte. Use a
size of MAX_LENGTH_OF_LONG to fix this issue instead of a fixed size
of 11 chars.

Closes GH-10525.
  • Loading branch information
nielsdos committed Mar 20, 2023
1 parent c407243 commit 3014182
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 4 deletions.
2 changes: 2 additions & 0 deletions NEWS
Expand Up @@ -24,6 +24,8 @@ PHP NEWS

- FTP:
. Propagate success status of ftp_close(). (nielsdos)
. Fixed bug GH-10521 (ftp_get/ftp_nb_get resumepos offset is maximum 10GB).
(nielsdos)

- Opcache:
. Fixed build for macOS to cater with pkg-config settings. (David Carlier)
Expand Down
8 changes: 4 additions & 4 deletions ext/ftp/ftp.c
Expand Up @@ -867,7 +867,7 @@ ftp_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_t pat
{
databuf_t *data = NULL;
size_t rcvd;
char arg[11];
char arg[MAX_LENGTH_OF_LONG];

if (ftp == NULL) {
return 0;
Expand Down Expand Up @@ -964,7 +964,7 @@ ftp_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *inst
zend_long size;
char *ptr;
int ch;
char arg[11];
char arg[MAX_LENGTH_OF_LONG];

if (ftp == NULL) {
return 0;
Expand Down Expand Up @@ -2057,7 +2057,7 @@ int
ftp_nb_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_t path_len, ftptype_t type, zend_long resumepos)
{
databuf_t *data = NULL;
char arg[11];
char arg[MAX_LENGTH_OF_LONG];

if (ftp == NULL) {
return PHP_FTP_FAILED;
Expand Down Expand Up @@ -2176,7 +2176,7 @@ int
ftp_nb_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *instream, ftptype_t type, zend_long startpos)
{
databuf_t *data = NULL;
char arg[11];
char arg[MAX_LENGTH_OF_LONG];

if (ftp == NULL) {
return 0;
Expand Down
39 changes: 39 additions & 0 deletions ext/ftp/tests/gh10521.phpt
@@ -0,0 +1,39 @@
--TEST--
GH-10521 (ftp_get/ftp_nb_get resumepos offset is maximum 10GB)
--EXTENSIONS--
ftp
pcntl
--SKIPIF--
<?php
if (PHP_INT_SIZE != 8) die("skip: 64-bit only");
?>
--FILE--
<?php
require 'server.inc';

$ftp = ftp_connect('127.0.0.1', $port);
if (!$ftp) die("Couldn't connect to the server");

var_dump(ftp_login($ftp, 'anonymous', 'IEUser@'));

$local_file = __DIR__ . DIRECTORY_SEPARATOR . "gh10521.txt";

foreach ([12345678910, 9223372036854775807] as $size) {
$handle = fopen($local_file, 'w');
// Doesn't actually succeed in transferring a file. The file transfer gets aborted by our fake server.
// We just want to see if the offset was correctly received.
ftp_fget($ftp, $handle, 'gh10521', FTP_ASCII, $size);
fclose($handle);
}

?>
--CLEAN--
<?php
@unlink(__DIR__ . DIRECTORY_SEPARATOR . "gh10521.txt");
?>
--EXPECTF--
bool(true)

%s: ftp_fget(): Can't open data connection (12345678910). in %s on line %d

%s: ftp_fget(): Can't open data connection (9223372036854775807). in %s on line %d
4 changes: 4 additions & 0 deletions ext/ftp/tests/server.inc
Expand Up @@ -387,6 +387,10 @@ if ($pid) {
case "/bug73457":
fputs($s, "150 File status okay; about to open data connection.\r\n");
break;
case "gh10521":
// Just a side channel for getting the received file size.
fputs($s, "425 Can't open data connection (".$GLOBALS['rest_pos'].").\r\n");
break;

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

0 comments on commit 3014182

Please sign in to comment.