Skip to content

Commit 95a4724

Browse files
iliaaliliaal
committed
Merge branch 'PHP-8.5'
* PHP-8.5: Fix use-after-free on re-entrant ftp_close() during a transfer
2 parents 75cdbcd + 765fe90 commit 95a4724

7 files changed

Lines changed: 253 additions & 3 deletions

‎ext/ftp/ftp.c‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,11 @@ bool ftp_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_
817817
if (ftp == NULL) {
818818
return false;
819819
}
820+
if (ftp->in_use) {
821+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
822+
return false;
823+
}
824+
ftp->in_use = true;
820825
if (!ftp_type(ftp, type)) {
821826
goto bail;
822827
}
@@ -913,9 +918,11 @@ bool ftp_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_
913918
goto bail;
914919
}
915920

921+
ftp->in_use = false;
916922
return true;
917923
bail:
918924
data_close(ftp);
925+
ftp->in_use = false;
919926
return false;
920927
}
921928

@@ -1000,6 +1007,11 @@ bool ftp_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream
10001007
if (ftp == NULL) {
10011008
return false;
10021009
}
1010+
if (ftp->in_use) {
1011+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
1012+
return false;
1013+
}
1014+
ftp->in_use = true;
10031015
if (!ftp_type(ftp, type)) {
10041016
goto bail;
10051017
}
@@ -1040,9 +1052,11 @@ bool ftp_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream
10401052
if (!ftp_getresp(ftp) || (ftp->resp != 226 && ftp->resp != 250 && ftp->resp != 200)) {
10411053
goto bail;
10421054
}
1055+
ftp->in_use = false;
10431056
return true;
10441057
bail:
10451058
data_close(ftp);
1059+
ftp->in_use = false;
10461060
return false;
10471061
}
10481062

@@ -1053,6 +1067,11 @@ bool ftp_append(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stre
10531067
if (ftp == NULL) {
10541068
return false;
10551069
}
1070+
if (ftp->in_use) {
1071+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
1072+
return false;
1073+
}
1074+
ftp->in_use = true;
10561075
if (!ftp_type(ftp, type)) {
10571076
goto bail;
10581077
}
@@ -1080,9 +1099,11 @@ bool ftp_append(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stre
10801099
if (!ftp_getresp(ftp) || (ftp->resp != 226 && ftp->resp != 250 && ftp->resp != 200)) {
10811100
goto bail;
10821101
}
1102+
ftp->in_use = false;
10831103
return true;
10841104
bail:
10851105
data_close(ftp);
1106+
ftp->in_use = false;
10861107
return false;
10871108
}
10881109

@@ -1939,6 +1960,10 @@ static char** ftp_genlist(ftpbuf_t *ftp, const char *cmd, const size_t cmd_len,
19391960
char **entry;
19401961
char *text;
19411962

1963+
if (ftp->in_use) {
1964+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
1965+
return NULL;
1966+
}
19421967

19431968
if ((tmpstream = php_stream_fopen_tmpfile()) == NULL) {
19441969
php_error_docref(NULL, E_WARNING, "Unable to create temporary file. Check permissions in temporary files directory.");
@@ -2037,6 +2062,11 @@ int ftp_nb_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const siz
20372062
return PHP_FTP_FAILED;
20382063
}
20392064

2065+
if (ftp->in_use) {
2066+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
2067+
return PHP_FTP_FAILED;
2068+
}
2069+
20402070
if (ftp->data != NULL) {
20412071
/* If there is a transfer in action, abort it.
20422072
* If we don't, we get an invalid state and memory leaks when the new connection gets opened. */
@@ -2101,11 +2131,17 @@ int ftp_nb_continue_read(ftpbuf_t *ftp)
21012131

21022132
data = ftp->data;
21032133

2134+
if (ftp->in_use) {
2135+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
2136+
return PHP_FTP_FAILED;
2137+
}
2138+
21042139
/* check if there is already more data */
21052140
if (!data_available(ftp, data->fd, false)) {
21062141
return PHP_FTP_MOREDATA;
21072142
}
21082143

2144+
ftp->in_use = true;
21092145
type = ftp->type;
21102146

21112147
lastch = ftp->lastch;
@@ -2129,6 +2165,7 @@ int ftp_nb_continue_read(ftpbuf_t *ftp)
21292165
}
21302166

21312167
ftp->lastch = lastch;
2168+
ftp->in_use = false;
21322169
return PHP_FTP_MOREDATA;
21332170
}
21342171

@@ -2143,9 +2180,11 @@ int ftp_nb_continue_read(ftpbuf_t *ftp)
21432180
}
21442181

21452182
ftp->nb = false;
2183+
ftp->in_use = false;
21462184
return PHP_FTP_FINISHED;
21472185
bail:
21482186
ftp->nb = false;
2187+
ftp->in_use = false;
21492188
data_close(ftp);
21502189
return PHP_FTP_FAILED;
21512190
}
@@ -2158,6 +2197,10 @@ int ftp_nb_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_strea
21582197
if (ftp == NULL) {
21592198
return 0;
21602199
}
2200+
if (ftp->in_use) {
2201+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
2202+
return PHP_FTP_FAILED;
2203+
}
21612204
if (!ftp_type(ftp, type)) {
21622205
goto bail;
21632206
}
@@ -2201,16 +2244,24 @@ int ftp_nb_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_strea
22012244

22022245
int ftp_nb_continue_write(ftpbuf_t *ftp)
22032246
{
2247+
if (ftp->in_use) {
2248+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
2249+
return PHP_FTP_FAILED;
2250+
}
2251+
22042252
/* check if we can write more data */
22052253
if (!data_writeable(ftp, ftp->data->fd)) {
22062254
return PHP_FTP_MOREDATA;
22072255
}
22082256

2257+
ftp->in_use = true;
2258+
22092259
if (ftp_send_stream_to_data_socket(ftp, ftp->data, ftp->stream, ftp->type, true) != SUCCESS) {
22102260
goto bail;
22112261
}
22122262

22132263
if (!php_stream_eof(ftp->stream)) {
2264+
ftp->in_use = false;
22142265
return PHP_FTP_MOREDATA;
22152266
}
22162267

@@ -2220,9 +2271,11 @@ int ftp_nb_continue_write(ftpbuf_t *ftp)
22202271
goto bail;
22212272
}
22222273
ftp->nb = false;
2274+
ftp->in_use = false;
22232275
return PHP_FTP_FINISHED;
22242276
bail:
22252277
data_close(ftp);
22262278
ftp->nb = false;
2279+
ftp->in_use = false;
22272280
return PHP_FTP_FAILED;
22282281
}

‎ext/ftp/ftp.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ typedef struct ftpbuf
7070
databuf_t *data; /* Data connection for "nonblocking" transfers */
7171
php_stream *stream; /* output stream for "nonblocking" transfers */
7272
bool nb; /* "nonblocking" transfer in progress */
73+
bool in_use; /* engine transfer in progress; blocks re-entrant ftp_close */
7374
char lastch; /* last char of previous call */
7475
bool direction; /* recv = 0 / send = 1 */
7576
bool closestream;/* close or not close stream */

‎ext/ftp/php_ftp.c‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,11 @@ PHP_FUNCTION(ftp_nb_fget)
650650
}
651651

652652
/* configuration */
653+
if (ftp->in_use) {
654+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
655+
RETURN_FALSE;
656+
}
657+
653658
ftp->direction = 0; /* recv */
654659
ftp->closestream = 0; /* do not close */
655660

@@ -760,6 +765,10 @@ PHP_FUNCTION(ftp_nb_get)
760765
RETURN_THROWS();
761766
}
762767
GET_FTPBUF(ftp, z_ftp);
768+
if (ftp->in_use) {
769+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
770+
RETURN_FALSE;
771+
}
763772
XTYPE(xtype, mode);
764773

765774
/* ignore autoresume if autoseek is switched off */
@@ -797,8 +806,8 @@ PHP_FUNCTION(ftp_nb_get)
797806
ftp->closestream = true; /* do close */
798807

799808
if ((ret = ftp_nb_get(ftp, outstream, remote, remote_len, xtype, resumepos)) == PHP_FTP_FAILED) {
800-
php_stream_close(outstream);
801809
ftp->stream = NULL;
810+
php_stream_close(outstream);
802811
VCWD_UNLINK(local);
803812
if (*ftp->inbuf) {
804813
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
@@ -807,8 +816,8 @@ PHP_FUNCTION(ftp_nb_get)
807816
}
808817

809818
if (ret == PHP_FTP_FINISHED){
810-
php_stream_close(outstream);
811819
ftp->stream = NULL;
820+
php_stream_close(outstream);
812821
}
813822

814823
RETURN_LONG(ret);
@@ -946,6 +955,11 @@ PHP_FUNCTION(ftp_nb_fput)
946955
}
947956

948957
/* configuration */
958+
if (ftp->in_use) {
959+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
960+
RETURN_FALSE;
961+
}
962+
949963
ftp->direction = true; /* send */
950964
ftp->closestream = false; /* do not close */
951965

@@ -1086,15 +1100,21 @@ PHP_FUNCTION(ftp_nb_put)
10861100
}
10871101
}
10881102

1103+
if (ftp->in_use) {
1104+
php_stream_close(instream);
1105+
php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
1106+
RETURN_FALSE;
1107+
}
1108+
10891109
/* configuration */
10901110
ftp->direction = true; /* send */
10911111
ftp->closestream = true; /* do close */
10921112

10931113
ret = ftp_nb_put(ftp, remote, remote_len, instream, xtype, startpos);
10941114

10951115
if (ret != PHP_FTP_MOREDATA) {
1096-
php_stream_close(instream);
10971116
ftp->stream = NULL;
1117+
php_stream_close(instream);
10981118
}
10991119

11001120
if (ret == PHP_FTP_FAILED) {
@@ -1229,6 +1249,10 @@ PHP_FUNCTION(ftp_close)
12291249

12301250
obj = ftp_object_from_zend_object(Z_OBJ_P(z_ftp));
12311251
if (obj->ftp) {
1252+
if (obj->ftp->in_use) {
1253+
zend_throw_error(NULL, "Cannot close FTP\\Connection while a transfer is in progress");
1254+
RETURN_THROWS();
1255+
}
12321256
success = ftp_quit(obj->ftp);
12331257
ftp_close(obj->ftp);
12341258
obj->ftp = NULL;

‎ext/ftp/tests/ftp_close_during_transfer.phpt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
ftp_close() from a stream wrapper during a transfer throws instead of freeing the connection
3+
--EXTENSIONS--
4+
ftp
5+
pcntl
6+
--FILE--
7+
<?php
8+
require 'server.inc';
9+
10+
class CloseDuringWrite {
11+
public $context;
12+
public static $ftp;
13+
public function stream_open($path, $mode, $options, &$opened_path) {
14+
return true;
15+
}
16+
public function stream_write($data) {
17+
ftp_close(self::$ftp);
18+
return strlen($data);
19+
}
20+
public function stream_close() {}
21+
public function stream_eof() {
22+
return true;
23+
}
24+
}
25+
26+
stream_wrapper_register('reentrant', CloseDuringWrite::class);
27+
28+
$ftp = ftp_connect('127.0.0.1', $port);
29+
var_dump(ftp_login($ftp, 'user', 'pass'));
30+
CloseDuringWrite::$ftp = $ftp;
31+
32+
try {
33+
@ftp_get($ftp, 'reentrant://sink', 'a story.txt', FTP_BINARY);
34+
} catch (\Error $e) {
35+
echo $e->getMessage(), "\n";
36+
}
37+
38+
ftp_close($ftp);
39+
echo "closed\n";
40+
?>
41+
--EXPECT--
42+
bool(true)
43+
Cannot close FTP\Connection while a transfer is in progress
44+
closed

‎ext/ftp/tests/ftp_nb_close_during_transfer.phpt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
ftp_close() from a stream wrapper during a non-blocking transfer throws instead of freeing the connection
3+
--EXTENSIONS--
4+
ftp
5+
pcntl
6+
--FILE--
7+
<?php
8+
require 'server.inc';
9+
10+
class CloseDuringNbWrite {
11+
public $context;
12+
public static $ftp;
13+
public function stream_open($path, $mode, $options, &$opened_path) {
14+
return true;
15+
}
16+
public function stream_write($data) {
17+
ftp_close(self::$ftp);
18+
return strlen($data);
19+
}
20+
public function stream_close() {}
21+
public function stream_eof() {
22+
return true;
23+
}
24+
}
25+
26+
stream_wrapper_register('reentrantnb', CloseDuringNbWrite::class);
27+
28+
$ftp = ftp_connect('127.0.0.1', $port);
29+
var_dump(ftp_login($ftp, 'user', 'pass'));
30+
CloseDuringNbWrite::$ftp = $ftp;
31+
32+
try {
33+
@ftp_nb_get($ftp, 'reentrantnb://sink', 'a story.txt', FTP_BINARY);
34+
} catch (\Error $e) {
35+
echo $e->getMessage(), "\n";
36+
}
37+
38+
ftp_close($ftp);
39+
echo "closed\n";
40+
?>
41+
--EXPECT--
42+
bool(true)
43+
Cannot close FTP\Connection while a transfer is in progress
44+
closed

0 commit comments

Comments
 (0)