Skip to content

Commit 274cbec

Browse files
committed
ext/ftp: apply the connect timeout ceiling to the remaining entry points
GH-20601 capped the timeout in ftp_connect() so a value that overflows the timeval conversion cannot reach php_network_set_limit_time(). ftp_ssl_connect() and ftp_set_option(FTP_TIMEOUT_SEC) kept rejecting only non-positive values, so PHP_INT_MAX still reached ftp_open() and my_poll() through them. Apply the same bound to both. ftp_ssl_connect() with PHP_INT_MAX hung before this rather than reporting the bad argument.
1 parent 5fd38ba commit 274cbec

3 files changed

Lines changed: 59 additions & 0 deletions

File tree

ext/ftp/php_ftp.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,18 @@ PHP_FUNCTION(ftp_ssl_connect)
191191
RETURN_THROWS();
192192
}
193193

194+
const uint64_t timeoutmax = (uint64_t)((double) PHP_TIMEOUT_ULL_MAX / 1000000.0);
195+
194196
if (timeout_sec <= 0) {
195197
zend_argument_value_error(3, "must be greater than 0");
196198
RETURN_THROWS();
197199
}
198200

201+
if (timeout_sec >= timeoutmax) {
202+
zend_argument_value_error(3, "must be less than " ZEND_ULONG_FMT, timeoutmax);
203+
RETURN_THROWS();
204+
}
205+
199206
/* connect */
200207
if (!(ftp = ftp_open(host, (short)port, timeout_sec))) {
201208
RETURN_FALSE;
@@ -1259,6 +1266,7 @@ PHP_FUNCTION(ftp_set_option)
12591266
zval *z_ftp, *z_value;
12601267
zend_long option;
12611268
ftpbuf_t *ftp;
1269+
const uint64_t timeoutmax = (uint64_t)((double) PHP_TIMEOUT_ULL_MAX / 1000000.0);
12621270

12631271
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Olz", &z_ftp, php_ftp_ce, &option, &z_value) == FAILURE) {
12641272
RETURN_THROWS();
@@ -1275,6 +1283,10 @@ PHP_FUNCTION(ftp_set_option)
12751283
zend_argument_value_error(3, "must be greater than 0 for the FTP_TIMEOUT_SEC option");
12761284
RETURN_THROWS();
12771285
}
1286+
if ((uint64_t) Z_LVAL_P(z_value) >= timeoutmax) {
1287+
zend_argument_value_error(3, "must be less than " ZEND_ULONG_FMT " for the FTP_TIMEOUT_SEC option", timeoutmax);
1288+
RETURN_THROWS();
1289+
}
12781290
ftp->timeout_sec = Z_LVAL_P(z_value);
12791291
RETURN_TRUE;
12801292
break;
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-20601 (ftp_set_option FTP_TIMEOUT_SEC timeout overflow)
3+
--EXTENSIONS--
4+
ftp
5+
pcntl
6+
--SKIPIF--
7+
<?php
8+
if (PHP_INT_SIZE != 8) die("skip: 64-bit only");
9+
if (PHP_OS_FAMILY === 'Windows') die("skip not for windows");
10+
?>
11+
--FILE--
12+
<?php
13+
require 'server.inc';
14+
15+
$ftp = ftp_connect('127.0.0.1', $port);
16+
ftp_login($ftp, 'user', 'pass');
17+
$ftp or die("Couldn't connect to the server");
18+
19+
try {
20+
ftp_set_option($ftp, FTP_TIMEOUT_SEC, PHP_INT_MAX);
21+
} catch (\ValueError $e) {
22+
echo $e->getMessage();
23+
}
24+
?>
25+
--EXPECTF--
26+
ftp_set_option(): Argument #3 ($value) must be less than %d for the FTP_TIMEOUT_SEC option

ext/ftp/tests/gh20601_ssl.phpt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
GH-20601 (ftp_ssl_connect timeout overflow)
3+
--EXTENSIONS--
4+
ftp
5+
openssl
6+
--SKIPIF--
7+
<?php
8+
if (!function_exists("ftp_ssl_connect")) die("skip ftp_ssl is disabled");
9+
if (PHP_INT_SIZE != 8) die("skip: 64-bit only");
10+
if (PHP_OS_FAMILY === 'Windows') die("skip not for windows");
11+
?>
12+
--FILE--
13+
<?php
14+
try {
15+
ftp_ssl_connect('127.0.0.1', 1024, PHP_INT_MAX);
16+
} catch (\ValueError $e) {
17+
echo $e->getMessage();
18+
}
19+
?>
20+
--EXPECTF--
21+
ftp_ssl_connect(): Argument #3 ($timeout) must be less than %d

0 commit comments

Comments
 (0)