Skip to content

Fix use-after-free on re-entrant ftp_close() during a transfer #22400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
iliaal wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
+133 −0
Open

Fix use-after-free on re-entrant ftp_close() during a transfer #22400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions ext/ftp/ftp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,11 @@ ftp_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_t pat
if (ftp == NULL) {
return 0;
}
if (ftp->in_use) {
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
return 0;
}
ftp->in_use = true;
if (!ftp_type(ftp, type)) {
goto bail;
}
Expand Down Expand Up @@ -967,9 +972,11 @@ ftp_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_t pat
goto bail;
}

ftp->in_use = false;
return 1;
bail:
data_close(ftp);
ftp->in_use = false;
return 0;
}
/* }}} */
Expand Down Expand Up @@ -1057,6 +1064,11 @@ ftp_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *inst
if (ftp == NULL) {
return 0;
}
if (ftp->in_use) {
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
return 0;
}
ftp->in_use = true;
if (!ftp_type(ftp, type)) {
goto bail;
}
Expand Down Expand Up @@ -1097,9 +1109,11 @@ ftp_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *inst
if (!ftp_getresp(ftp) || (ftp->resp != 226 && ftp->resp != 250 && ftp->resp != 200)) {
goto bail;
}
ftp->in_use = false;
return 1;
bail:
data_close(ftp);
ftp->in_use = false;
return 0;
}
/* }}} */
Expand All @@ -1114,6 +1128,11 @@ ftp_append(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *i
if (ftp == NULL) {
return 0;
}
if (ftp->in_use) {
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
return 0;
}
ftp->in_use = true;
if (!ftp_type(ftp, type)) {
goto bail;
}
Expand Down Expand Up @@ -1141,9 +1160,11 @@ ftp_append(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *i
if (!ftp_getresp(ftp) || (ftp->resp != 226 && ftp->resp != 250 && ftp->resp != 200)) {
goto bail;
}
ftp->in_use = false;
return 1;
bail:
data_close(ftp);
ftp->in_use = false;
return 0;
}
/* }}} */
Expand Down Expand Up @@ -2223,11 +2244,17 @@ ftp_nb_continue_read(ftpbuf_t *ftp)

data = ftp->data;

if (ftp->in_use) {
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
return PHP_FTP_FAILED;
}

/* check if there is already more data */
if (!data_available(ftp, data->fd, false)) {
return PHP_FTP_MOREDATA;
}

ftp->in_use = true;
type = ftp->type;

lastch = ftp->lastch;
Expand All @@ -2251,6 +2278,7 @@ ftp_nb_continue_read(ftpbuf_t *ftp)
}

ftp->lastch = lastch;
ftp->in_use = false;
return PHP_FTP_MOREDATA;
}

Expand All @@ -2265,9 +2293,11 @@ ftp_nb_continue_read(ftpbuf_t *ftp)
}

ftp->nb = 0;
ftp->in_use = false;
return PHP_FTP_FINISHED;
bail:
ftp->nb = 0;
ftp->in_use = false;
data_close(ftp);
return PHP_FTP_FAILED;
}
Expand Down Expand Up @@ -2330,16 +2360,24 @@ ftp_nb_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *i
int
ftp_nb_continue_write(ftpbuf_t *ftp)
{
if (ftp->in_use) {
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
return PHP_FTP_FAILED;
}

/* check if we can write more data */
if (!data_writeable(ftp, ftp->data->fd)) {
return PHP_FTP_MOREDATA;
}

ftp->in_use = true;

if (ftp_send_stream_to_data_socket(ftp, ftp->data, ftp->stream, ftp->type, true) != SUCCESS) {
goto bail;
}

if (!php_stream_eof(ftp->stream)) {
ftp->in_use = false;
return PHP_FTP_MOREDATA;
}

Expand All @@ -2349,10 +2387,12 @@ ftp_nb_continue_write(ftpbuf_t *ftp)
goto bail;
}
ftp->nb = 0;
ftp->in_use = false;
return PHP_FTP_FINISHED;
bail:
data_close(ftp);
ftp->nb = 0;
ftp->in_use = false;
return PHP_FTP_FAILED;
}
/* }}} */
1 change: 1 addition & 0 deletions ext/ftp/ftp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ typedef struct ftpbuf
databuf_t *data; /* Data connection for "nonblocking" transfers */
php_stream *stream; /* output stream for "nonblocking" transfers */
bool nb; /* "nonblocking" transfer in progress */
bool in_use; /* engine transfer in progress; blocks re-entrant ftp_close */
char lastch; /* last char of previous call */
bool direction; /* recv = 0 / send = 1 */
bool closestream;/* close or not close stream */
Expand Down
4 changes: 4 additions & 0 deletions ext/ftp/php_ftp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,10 @@ PHP_FUNCTION(ftp_close)

obj = ftp_object_from_zend_object(Z_OBJ_P(z_ftp));
if (obj->ftp) {
if (obj->ftp->in_use) {
zend_throw_error(NULL, "Cannot close FTP\\Connection while a transfer is in progress");
RETURN_THROWS();
}
success = ftp_quit(obj->ftp);
ftp_close(obj->ftp);
obj->ftp = NULL;
Expand Down
44 changes: 44 additions & 0 deletions ext/ftp/tests/ftp_close_during_transfer.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
--TEST--
ftp_close() from a stream wrapper during a transfer throws instead of freeing the connection
--EXTENSIONS--
ftp
pcntl
--FILE--
<?php
require 'server.inc';

class CloseDuringWrite {
public $context;
public static $ftp;
public function stream_open($path, $mode, $options, &$opened_path) {
return true;
}
public function stream_write($data) {
ftp_close(self::$ftp);
return strlen($data);
}
public function stream_close() {}
public function stream_eof() {
return true;
}
}

stream_wrapper_register('reentrant', CloseDuringWrite::class);

$ftp = ftp_connect('127.0.0.1', $port);
var_dump(ftp_login($ftp, 'user', 'pass'));
CloseDuringWrite::$ftp = $ftp;

try {
@ftp_get($ftp, 'reentrant://sink', 'a story.txt', FTP_BINARY);
} catch (\Error $e) {
echo $e->getMessage(), "\n";
}

ftp_close($ftp);
echo "closed\n";
?>
--EXPECT--
bool(true)
Cannot close FTP\Connection while a transfer is in progress
closed
44 changes: 44 additions & 0 deletions ext/ftp/tests/ftp_nb_close_during_transfer.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
--TEST--
ftp_close() from a stream wrapper during a non-blocking transfer throws instead of freeing the connection
--EXTENSIONS--
ftp
pcntl
--FILE--
<?php
require 'server.inc';

class CloseDuringNbWrite {
public $context;
public static $ftp;
public function stream_open($path, $mode, $options, &$opened_path) {
return true;
}
public function stream_write($data) {
ftp_close(self::$ftp);
return strlen($data);
}
public function stream_close() {}
public function stream_eof() {
return true;
}
}

stream_wrapper_register('reentrantnb', CloseDuringNbWrite::class);

$ftp = ftp_connect('127.0.0.1', $port);
var_dump(ftp_login($ftp, 'user', 'pass'));
CloseDuringNbWrite::$ftp = $ftp;

try {
@ftp_nb_get($ftp, 'reentrantnb://sink', 'a story.txt', FTP_BINARY);
} catch (\Error $e) {
echo $e->getMessage(), "\n";
}

ftp_close($ftp);
echo "closed\n";
?>
--EXPECT--
bool(true)
Cannot close FTP\Connection while a transfer is in progress
closed
Loading