[php-src] master: Fix use-after-free on re-entrant ftp_close() during a transfer

Ilia Alshanetsky <[email protected]>
Newsgroups gmane.comp.php.cvs.general
Message-ID <[email protected]>
Author: Ilia Alshanetsky (iliaal)
Date: 2026-07-12T20:55:06-04:00

Commit: https://github.com/php/php-src/commit/1feb20156bbfc4450e455a92a026e5a53eba5937
Raw diff: https://github.com/php/php-src/commit/1feb20156bbfc4450e455a92a026e5a53eba5937.diff

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

Changed paths:
  A  ext/ftp/tests/ftp_close_during_transfer.phpt
  A  ext/ftp/tests/ftp_nb_close_during_transfer.phpt
  A  ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt
  A  ext/ftp/tests/ftp_nb_get_during_transfer.phpt
  M  ext/ftp/ftp.c
  M  ext/ftp/ftp.h
  M  ext/ftp/php_ftp.c


Diff:

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--
+<?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
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--
+<?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
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--
+<?php
+require 'server.inc';
+
+class NbGetDuringNbGet {
+    public $context;
+    public static $ftp;
+    public function stream_open($path, $mode, $options, &$opened_path) {
+        return true;
+    }
+    public function stream_write($data) {
+        @ftp_nb_get(self::$ftp, 'php://memory', 'a story.txt', FTP_BINARY);
+        return strlen($data);
+    }
+    public function stream_close() {}
+    public function stream_eof() {
+        return true;
+    }
+}
+
+stream_wrapper_register('reentrantnbget', NbGetDuringNbGet::class);
+
+$ftp = ftp_connect('127.0.0.1', $port);
+var_dump(ftp_login($ftp, 'user', 'pass'));
+NbGetDuringNbGet::$ftp = $ftp;
+
+$r = @ftp_nb_get($ftp, 'reentrantnbget://sink', 'a story.txt', FTP_BINARY);
+while ($r == FTP_MOREDATA) {
+    $r = @ftp_nb_continue($ftp);
+}
+var_dump($r === FTP_FINISHED);
+
+ftp_close($ftp);
+echo "closed\n";
+?>
+--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--
+<?php
+require 'server.inc';
+
+class NbGetDuringGet {
+    public $context;
+    public static $ftp;
+    public function stream_open($path, $mode, $options, &$opened_path) {
+        return true;
+    }
+    public function stream_write($data) {
+        @ftp_nb_get(self::$ftp, 'php://memory', 'a story.txt', FTP_BINARY);
+        return strlen($data);
+    }
+    public function stream_close() {}
+    public function stream_eof() {
+        return true;
+    }
+}
+
+stream_wrapper_register('reentrantget', NbGetDuringGet::class);
+
+$ftp = ftp_connect('127.0.0.1', $port);
+var_dump(ftp_login($ftp, 'user', 'pass'));
+NbGetDuringGet::$ftp = $ftp;
+
+var_dump(@ftp_get($ftp, 'reentrantget://sink', 'a story.txt', FTP_BINARY));
+
+ftp_close($ftp);
+echo "closed\n";
+?>
+--EXPECT--
+bool(true)
+bool(true)
+closed
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.