Skip to content

ext/sockets: socket_set_option switch from convert_to_long to zval_tr… #17135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

ext/sockets: socket_set_option switch from convert_to_long to zval_tr… #17135

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0e4eeda
ext/sockets: socket_set_option switch from convert_to_long to zval_tr…
devnexen Dec 12, 2024
45270c6
add test
devnexen Dec 12, 2024
695861e
changes from review
devnexen Dec 13, 2024
6c5f436
add test case
devnexen Dec 13, 2024
1a14443
changes from review also attempt to prevent UB with timeout on windows.
devnexen Dec 13, 2024
e18fbb3
remove UB check and using zval_get_long instead
devnexen Dec 13, 2024
d6e64e6
other changes from review
devnexen Dec 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 70 additions & 16 deletions ext/sockets/sockets.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1811,6 +1811,7 @@ PHP_FUNCTION(socket_set_option)
HashTable *opt_ht;
zval *l_onoff, *l_linger;
zval *sec, *usec;
bool failed;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "Ollz", &arg1, socket_ce, &level, &optname, &arg4) == FAILURE) {
RETURN_THROWS();
Expand Down Expand Up @@ -1871,7 +1872,11 @@ PHP_FUNCTION(socket_set_option)
const char l_onoff_key[] = "l_onoff";
const char l_linger_key[] = "l_linger";

convert_to_array(arg4);
if (Z_TYPE_P(arg4) != IS_ARRAY) {
zend_argument_type_error(4, "must be of type array, %s given", zend_zval_value_name(arg4));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding what option we are talking about would help the error message.

Suggested change
zend_argument_type_error(4, "must be of type array, %s given", zend_zval_value_name(arg4));
zend_argument_type_error(4, "must be of type array when argument #3 ($option) is SO_LINGER, %s given", zend_zval_value_name(arg4));

And ditto for the other error cases.

RETURN_THROWS();
}

opt_ht = Z_ARRVAL_P(arg4);

if ((l_onoff = zend_hash_str_find(opt_ht, l_onoff_key, sizeof(l_onoff_key) - 1)) == NULL) {
Expand All @@ -1883,11 +1888,29 @@ PHP_FUNCTION(socket_set_option)
RETURN_THROWS();
}

convert_to_long(l_onoff);
convert_to_long(l_linger);
zend_long vall_lonoff = zval_try_get_long(l_onoff, &failed);
if (failed) {
zend_argument_type_error(4, "\"%s\" must be of type int, %s given", l_onoff_key, zend_zval_value_name(l_onoff));
RETURN_THROWS();
}
zend_long vall_linger = zval_try_get_long(l_linger, &failed);
if (failed) {
zend_argument_type_error(4, "\"%s\" must be of type int, %s given", l_linger_key, zend_zval_value_name(l_linger));
RETURN_THROWS();
}

lv.l_onoff = (unsigned short)Z_LVAL_P(l_onoff);
lv.l_linger = (unsigned short)Z_LVAL_P(l_linger);
if (vall_lonoff < 0 || vall_lonoff > USHRT_MAX) {
zend_argument_value_error(4, "\"%s\" must be between 0 and %u", l_onoff_key, USHRT_MAX);
RETURN_THROWS();
}

if (vall_linger < 0 || vall_linger > USHRT_MAX) {
zend_argument_value_error(4, "\"%s\" must be between 0 and %d", l_linger, USHRT_MAX);
RETURN_THROWS();
}

lv.l_onoff = (unsigned short)vall_lonoff;
lv.l_linger = (unsigned short)vall_linger;

optlen = sizeof(lv);
opt_ptr = &lv;
Expand All @@ -1899,7 +1922,11 @@ PHP_FUNCTION(socket_set_option)
const char sec_key[] = "sec";
const char usec_key[] = "usec";

convert_to_array(arg4);
if (Z_TYPE_P(arg4) != IS_ARRAY) {
zend_argument_type_error(4, "must be of type array, %s given", zend_zval_value_name(arg4));
RETURN_THROWS();
}

opt_ht = Z_ARRVAL_P(arg4);

if ((sec = zend_hash_str_find(opt_ht, sec_key, sizeof(sec_key) - 1)) == NULL) {
Expand All @@ -1911,15 +1938,35 @@ PHP_FUNCTION(socket_set_option)
RETURN_THROWS();
}

convert_to_long(sec);
convert_to_long(usec);
zend_long valsec = zval_try_get_long(sec, &failed);
if (failed) {
zend_argument_type_error(4, "\"%s\" must be of type int, %s given", sec_key, zend_zval_value_name(sec));
RETURN_THROWS();
}
zend_long valusec = zval_try_get_long(usec, &failed);
if (failed) {
zend_argument_type_error(4, "\"%s\" must be of type int, %s given", usec_key, zend_zval_value_name(usec));
RETURN_THROWS();
}
#ifndef PHP_WIN32
tv.tv_sec = Z_LVAL_P(sec);
tv.tv_usec = Z_LVAL_P(usec);
tv.tv_sec = valsec;
tv.tv_usec = valusec;
optlen = sizeof(tv);
opt_ptr = &tv;
#else
timeout = Z_LVAL_P(sec) * 1000 + Z_LVAL_P(usec) / 1000;
if (valsec < 0 || valsec > INT_MAX / 1000) {
zend_argument_value_error(4, "\"%s\" must be between 0 and %d", sec_key, INT_MAX / 1000);
RETURN_THROWS();
}

timeout = valsec * 1000;

if (valusec < 0 || timeout > INT_MAX - (valusec / 1000)) {
zend_argument_value_error(4, "\"%s\" must be between 0 and %u", usec_key, (DWORD)(INT_MAX - (valusec / 1000)));
RETURN_THROWS();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this works for very large usec. We may also need to take BC into account (currently, negative usec are likely allowed for Windows). Maybe leave the overflow checking for master; while technically UB can happen, I think wrap-around is happening in practice (and nothing really bad).


timeout += valusec / 1000;
optlen = sizeof(int);
opt_ptr = &timeout;
#endif
Expand Down Expand Up @@ -1971,15 +2018,19 @@ PHP_FUNCTION(socket_set_option)

#ifdef SO_ATTACH_REUSEPORT_CBPF
case SO_ATTACH_REUSEPORT_CBPF: {
convert_to_long(arg4);
zend_long fval = zval_try_get_long(arg4, &failed);
if (failed) {
zend_argument_type_error(4, "must be of type int, %s given", zend_zval_value_name(arg4));
RETURN_THROWS();
}

if (!Z_LVAL_P(arg4)) {
if (!fval) {
ov = 1;
optlen = sizeof(ov);
opt_ptr = &ov;
optname = SO_DETACH_BPF;
} else {
uint32_t k = (uint32_t)Z_LVAL_P(arg4);
uint32_t k = (uint32_t)fval;
static struct sock_filter cbpf[8] = {0};
static struct sock_fprog bpfprog;

Expand All @@ -2006,8 +2057,11 @@ PHP_FUNCTION(socket_set_option)

default:
default_case:
convert_to_long(arg4);
ov = Z_LVAL_P(arg4);
ov = zval_try_get_long(arg4, &failed);
if (failed) {
zend_argument_type_error(4, "must be an int, %s given", zend_zval_value_name(arg4));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

RETURN_THROWS();
}

optlen = sizeof(ov);
opt_ptr = &ov;
Expand Down
6 changes: 6 additions & 0 deletions ext/sockets/tests/socket_reuseport_cbpf.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ if (!$socket) {
}
var_dump(socket_set_option( $socket, SOL_SOCKET, SO_REUSEADDR, true));
var_dump(socket_set_option( $socket, SOL_SOCKET, SO_REUSEPORT, true));
try {
socket_set_option( $socket, SOL_SOCKET, SO_ATTACH_REUSEPORT_CBPF, array());
} catch (\TypeError $e) {
echo $e->getMessage() . PHP_EOL;
}
var_dump(socket_set_option( $socket, SOL_SOCKET, SO_ATTACH_REUSEPORT_CBPF, SKF_AD_CPU));
var_dump(socket_bind($socket, '0.0.0.0'));
socket_listen($socket);
Expand All @@ -26,5 +31,6 @@ socket_close($socket);
--EXPECT--
bool(true)
bool(true)
socket_set_option(): Argument #4 ($value) must be of type int, array given
bool(true)
bool(true)
62 changes: 62 additions & 0 deletions ext/sockets/tests/socket_set_option_timeo_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
--TEST--
socket_set_option() with SO_RCVTIMEO/SO_SNDTIMEO/SO_LINGER
--EXTENSIONS--
sockets
--FILE--
<?php
$socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
if (!$socket) {
die('Unable to create AF_INET socket [socket]');
}
$options_1 = array("sec" => 1, "usec" => "aaaaa");
$options_2 = array("sec" => new stdClass(), "usec" => "1");
$options_3 = array("l_onoff" => "aaaa", "l_linger" => "1");
$options_4 = array("l_onoff" => "1", "l_linger" => []);
$options_5 = array("l_onoff" => PHP_INT_MAX, "l_linger" => "1");

try {
socket_set_option( $socket, SOL_SOCKET, SO_RCVTIMEO, new stdClass);
} catch (\TypeError $e) {
echo $e->getMessage() . PHP_EOL;
}

try {
socket_set_option( $socket, SOL_SOCKET, SO_RCVTIMEO, $options_1);
} catch (\TypeError $e) {
echo $e->getMessage() . PHP_EOL;
}

try {
socket_set_option( $socket, SOL_SOCKET, SO_SNDTIMEO, $options_2);
} catch (\TypeError $e) {
echo $e->getMessage() . PHP_EOL;
}
try {
socket_set_option( $socket, SOL_SOCKET, SO_LINGER, "not good");
} catch (\TypeError $e) {
echo $e->getMessage() . PHP_EOL;
}
try {
socket_set_option( $socket, SOL_SOCKET, SO_LINGER, $options_3);
} catch (\TypeError $e) {
echo $e->getMessage() . PHP_EOL;
}
try {
socket_set_option( $socket, SOL_SOCKET, SO_LINGER, $options_4);
} catch (\TypeError $e) {
echo $e->getMessage() . PHP_EOL;
}
try {
socket_set_option( $socket, SOL_SOCKET, SO_LINGER, $options_5);
} catch (\ValueError $e) {
echo $e->getMessage() . PHP_EOL;
}
?>
--EXPECTF--
socket_set_option(): Argument #4 ($value) must be of type array, stdClass given
socket_set_option(): Argument #4 ($value) "usec" must be of type int, string given
socket_set_option(): Argument #4 ($value) "sec" must be of type int, stdClass given
socket_set_option(): Argument #4 ($value) must be of type array, string given
socket_set_option(): Argument #4 ($value) "l_onoff" must be of type int, string given
socket_set_option(): Argument #4 ($value) "l_linger" must be of type int, array given
socket_set_option(): Argument #4 ($value) "l_onoff" must be between 0 and %d
Loading