From c962a96c347e1da517b859fd626e471d4eeb29e8 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 6 Jul 2023 21:07:52 +0200 Subject: [PATCH] Fix GH-10562: Memory leak and invalid state with consecutive ftp_nb_fget When the user does not fully consume the data stream, but instead opens a new one, a memory leak occurs. Moreover, the state is invalid: when more commands arrive they'll be handled out-of-sync because the state of the client does not match what the server is doing. This leads to all sorts of weirdness, for example: Warning: ftp_nb_fget(): OK. Fix it by gracefully closing the old data stream when a new data stream is started. Closes GH-11606. --- NEWS | 2 ++ ext/ftp/ftp.c | 9 +++++++++ ext/ftp/tests/gh10562.phpt | 40 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 ext/ftp/tests/gh10562.phpt diff --git a/NEWS b/NEWS index cd60985e00a..83c135c862b 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,8 @@ PHP NEWS - FTP: . Fix context option check for "overwrite". (JonasQuinten) + . Fixed bug GH-10562 (Memory leak and invalid state with consecutive + ftp_nb_fget). (nielsdos) - GD: . Fix most of the external libgd test failures. (Michael Orlitzky) diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c index 09367097cdb..9253edc5589 100644 --- a/ext/ftp/ftp.c +++ b/ext/ftp/ftp.c @@ -2063,6 +2063,15 @@ ftp_nb_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_t 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. */ + data_close(ftp, ftp->data); + if (!ftp_getresp(ftp) || (ftp->resp != 226 && ftp->resp != 250)) { + goto bail; + } + } + if (!ftp_type(ftp, type)) { goto bail; } diff --git a/ext/ftp/tests/gh10562.phpt b/ext/ftp/tests/gh10562.phpt new file mode 100644 index 00000000000..c2a4357521b --- /dev/null +++ b/ext/ftp/tests/gh10562.phpt @@ -0,0 +1,40 @@ +--TEST-- +GH-10562 (Memory leak with consecutive ftp_nb_fget) +--EXTENSIONS-- +ftp +pcntl +--FILE-- + +--CLEAN-- + +--EXPECT-- +bool(true) +bool(true) +BINARYFooBar +For sale: baby shoes, never worn.