From 2a27fcc3c550027e64bacae03ff481e22a1e9355 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 22 Jun 2026 10:27:06 -0400 Subject: [PATCH] Fix use-after-free on re-entrant ftp_close() during a transfer A user stream wrapper passed to a blocking or non-blocking FTP transfer can re-enter the engine from its stream_read/stream_write handler while the engine still holds the freed ftpbuf_t/databuf_t on the C stack, so the transfer resumes on freed memory. Guard every operation that opens a data connection with an in_use flag: ftp_close() throws while it is set, and the transfer and listing functions refuse to run. The non-blocking checks sit at the userland entry, before the wrapper mutates ftp->direction/stream, so a rejected re-entrant call cannot corrupt the outer transfer; the nb cleanup clears ftp->stream before closing the stream so a stream_close() that calls ftp_close() cannot double-close it. Closes GH-22400 --- ext/ftp/ftp.c | 53 +++++++++++++++++++ ext/ftp/ftp.h | 1 + ext/ftp/php_ftp.c | 30 +++++++++-- ext/ftp/tests/ftp_close_during_transfer.phpt | 44 +++++++++++++++ .../tests/ftp_nb_close_during_transfer.phpt | 44 +++++++++++++++ .../tests/ftp_nb_get_during_nb_transfer.phpt | 44 +++++++++++++++ ext/ftp/tests/ftp_nb_get_during_transfer.phpt | 40 ++++++++++++++ 7 files changed, 253 insertions(+), 3 deletions(-) create mode 100644 ext/ftp/tests/ftp_close_during_transfer.phpt create mode 100644 ext/ftp/tests/ftp_nb_close_during_transfer.phpt create mode 100644 ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt create mode 100644 ext/ftp/tests/ftp_nb_get_during_transfer.phpt diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c index 17a3e10e0d61..1345e378d8e3 100644 --- a/ext/ftp/ftp.c +++ b/ext/ftp/ftp.c @@ -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; } @@ -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; } /* }}} */ @@ -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; } @@ -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; } /* }}} */ @@ -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; } @@ -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; } /* }}} */ @@ -2055,6 +2076,10 @@ ftp_genlist(ftpbuf_t *ftp, const char *cmd, const size_t cmd_len, const char *pa char **entry; char *text; + if (ftp->in_use) { + php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); + return NULL; + } if ((tmpstream = php_stream_fopen_tmpfile()) == NULL) { php_error_docref(NULL, E_WARNING, "Unable to create temporary file. Check permissions in temporary files directory."); @@ -2156,6 +2181,11 @@ ftp_nb_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_t return PHP_FTP_FAILED; } + if (ftp->in_use) { + php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); + return PHP_FTP_FAILED; + } + if (ftp->data != NULL) { /* If there is a transfer in action, abort it. * If we don't, we get an invalid state and memory leaks when the new connection gets opened. */ @@ -2223,11 +2253,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; @@ -2251,6 +2287,7 @@ ftp_nb_continue_read(ftpbuf_t *ftp) } ftp->lastch = lastch; + ftp->in_use = false; return PHP_FTP_MOREDATA; } @@ -2265,9 +2302,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; } @@ -2283,6 +2322,10 @@ ftp_nb_put(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 PHP_FTP_FAILED; + } if (!ftp_type(ftp, type)) { goto bail; } @@ -2330,16 +2373,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; } @@ -2349,10 +2400,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; } /* }}} */ diff --git a/ext/ftp/ftp.h b/ext/ftp/ftp.h index 94abc588ca85..91aafad02b79 100644 --- a/ext/ftp/ftp.h +++ b/ext/ftp/ftp.h @@ -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 */ diff --git a/ext/ftp/php_ftp.c b/ext/ftp/php_ftp.c index 8fa7675c6e9d..432270cff190 100644 --- a/ext/ftp/php_ftp.c +++ b/ext/ftp/php_ftp.c @@ -648,6 +648,11 @@ PHP_FUNCTION(ftp_nb_fget) } /* configuration */ + if (ftp->in_use) { + php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); + RETURN_FALSE; + } + ftp->direction = 0; /* recv */ ftp->closestream = 0; /* do not close */ @@ -761,6 +766,10 @@ PHP_FUNCTION(ftp_nb_get) RETURN_THROWS(); } GET_FTPBUF(ftp, z_ftp); + if (ftp->in_use) { + php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); + RETURN_FALSE; + } XTYPE(xtype, mode); /* ignore autoresume if autoseek is switched off */ @@ -798,8 +807,8 @@ PHP_FUNCTION(ftp_nb_get) ftp->closestream = 1; /* do close */ if ((ret = ftp_nb_get(ftp, outstream, remote, remote_len, xtype, resumepos)) == PHP_FTP_FAILED) { - php_stream_close(outstream); ftp->stream = NULL; + php_stream_close(outstream); VCWD_UNLINK(local); if (*ftp->inbuf) { php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); @@ -808,8 +817,8 @@ PHP_FUNCTION(ftp_nb_get) } if (ret == PHP_FTP_FINISHED){ - php_stream_close(outstream); ftp->stream = NULL; + php_stream_close(outstream); } RETURN_LONG(ret); @@ -937,6 +946,11 @@ PHP_FUNCTION(ftp_nb_fput) } /* configuration */ + if (ftp->in_use) { + php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); + RETURN_FALSE; + } + ftp->direction = 1; /* send */ ftp->closestream = 0; /* do not close */ @@ -1077,6 +1091,12 @@ PHP_FUNCTION(ftp_nb_put) } } + if (ftp->in_use) { + php_stream_close(instream); + php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); + RETURN_FALSE; + } + /* configuration */ ftp->direction = 1; /* send */ ftp->closestream = 1; /* do close */ @@ -1084,8 +1104,8 @@ PHP_FUNCTION(ftp_nb_put) ret = ftp_nb_put(ftp, remote, remote_len, instream, xtype, startpos); if (ret != PHP_FTP_MOREDATA) { - php_stream_close(instream); ftp->stream = NULL; + php_stream_close(instream); } if (ret == PHP_FTP_FAILED) { @@ -1220,6 +1240,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; diff --git a/ext/ftp/tests/ftp_close_during_transfer.phpt b/ext/ftp/tests/ftp_close_during_transfer.phpt new file mode 100644 index 000000000000..72e410603921 --- /dev/null +++ b/ext/ftp/tests/ftp_close_during_transfer.phpt @@ -0,0 +1,44 @@ +--TEST-- +ftp_close() from a stream wrapper during a transfer throws instead of freeing the connection +--EXTENSIONS-- +ftp +pcntl +--FILE-- +getMessage(), "\n"; +} + +ftp_close($ftp); +echo "closed\n"; +?> +--EXPECT-- +bool(true) +Cannot close FTP\Connection while a transfer is in progress +closed diff --git a/ext/ftp/tests/ftp_nb_close_during_transfer.phpt b/ext/ftp/tests/ftp_nb_close_during_transfer.phpt new file mode 100644 index 000000000000..702648f69592 --- /dev/null +++ b/ext/ftp/tests/ftp_nb_close_during_transfer.phpt @@ -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-- +getMessage(), "\n"; +} + +ftp_close($ftp); +echo "closed\n"; +?> +--EXPECT-- +bool(true) +Cannot close FTP\Connection while a transfer is in progress +closed diff --git a/ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt b/ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt new file mode 100644 index 000000000000..deb1698c77c5 --- /dev/null +++ b/ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt @@ -0,0 +1,44 @@ +--TEST-- +Re-entrant ftp_nb_get() from a stream wrapper during an active non-blocking ftp_nb_get() must not corrupt the outer transfer +--EXTENSIONS-- +ftp +pcntl +--FILE-- + +--EXPECT-- +bool(true) +bool(true) +closed diff --git a/ext/ftp/tests/ftp_nb_get_during_transfer.phpt b/ext/ftp/tests/ftp_nb_get_during_transfer.phpt new file mode 100644 index 000000000000..c7920496feb4 --- /dev/null +++ b/ext/ftp/tests/ftp_nb_get_during_transfer.phpt @@ -0,0 +1,40 @@ +--TEST-- +Re-entrant ftp_nb_get() from a stream wrapper during a blocking ftp_get() must not free the active data connection +--EXTENSIONS-- +ftp +pcntl +--FILE-- + +--EXPECT-- +bool(true) +bool(true) +closed