diff --git a/NEWS b/NEWS index 4938b79b0713..f4ae526bc562 100644 --- a/NEWS +++ b/NEWS @@ -145,6 +145,10 @@ PHP NEWS . Fixed bug GH-12171 (SimpleXML property-proxy equality ignores element names). (Ilia Alshanetsky) +- SNMP: + . Fixed bug GH-25137 (SNMP accepts malformed remote ports). + (somethingwithproof) + - SOAP: . Fixed the SOAP client accepting a response body shorter than its Content-Length and reusing the connection. (Ilia Alshanetsky) diff --git a/UPGRADING b/UPGRADING index d9242ecb3c17..70b6647af201 100644 --- a/UPGRADING +++ b/UPGRADING @@ -44,6 +44,12 @@ PHP 8.7 UPGRADE NOTES ReflectionZendExtension, ReflectionAttribute and ReflectionConstant are now readonly. +- SNMP: + . A remote port in the hostname must now be one or more decimal digits + with no other characters. An empty, nonnumeric or partially numeric + port, such as "host:", "host:abc" or "host:123abc", now throws a + ValueError instead of being read as 0 or as its leading digits. + - SOAP: . SoapClient no longer follows an HTTP redirect to a different origin (scheme, host, or port) and throws a SoapFault instead. Previously the diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c index 2f0b34416ed1..566b85328caa 100644 --- a/ext/snmp/snmp.c +++ b/ext/snmp/snmp.c @@ -843,6 +843,30 @@ static bool php_snmp_parse_oid( } /* }}} */ +/* Parse a remote port: one or more decimal digits, nothing else, at most USHRT_MAX. */ +static bool snmp_parse_port(const char *port_string, unsigned short *port) +{ + unsigned int value = 0; + + if (*port_string == '\0') { + return false; + } + + do { + if (!ZEND_IS_DIGIT(*port_string)) { + return false; + } + + value = value * 10 + (unsigned int) (*port_string - '0'); + if (value > USHRT_MAX) { + return false; + } + } while (*++port_string != '\0'); + + *port = (unsigned short) value; + return true; +} + /* {{{ snmp_session_init allocates memory for session and session->peername, caller should free it manually using snmp_session_free() and efree() */ @@ -884,7 +908,6 @@ static bool snmp_session_init(php_snmp_session **session_p, int version, zend_st // TODO: Do not strip and re-add the port in peername? unsigned short remote_port = SNMP_PORT; - int tmp_port; *session_p = (php_snmp_session *)emalloc(sizeof(php_snmp_session)); session = *session_p; @@ -905,13 +928,10 @@ static bool snmp_session_init(php_snmp_session **session_p, int version, zend_st host_ptr++; if ((pptr = strchr(host_ptr, ']'))) { if (pptr[1] == ':') { - char *pport = pptr + 2; - tmp_port = atoi(pport); - if (tmp_port < 0 || tmp_port > USHRT_MAX) { + if (!snmp_parse_port(pptr + 2, &remote_port)) { zend_argument_value_error(hostname_arg_num, "remote port must be between 0 and %u", USHRT_MAX); return false; } - remote_port = (unsigned short)tmp_port; } *pptr = '\0'; } else { @@ -920,13 +940,10 @@ static bool snmp_session_init(php_snmp_session **session_p, int version, zend_st } } else { /* IPv4 address */ if ((pptr = strchr(host_ptr, ':'))) { - char *pport = pptr + 1; - tmp_port = atoi(pport); - if (tmp_port < 0 || tmp_port > USHRT_MAX) { + if (!snmp_parse_port(pptr + 1, &remote_port)) { zend_argument_value_error(hostname_arg_num, "remote port must be between 0 and %u", USHRT_MAX); return false; } - remote_port = (unsigned short)tmp_port; *pptr = '\0'; } } diff --git a/ext/snmp/tests/gh25137.phpt b/ext/snmp/tests/gh25137.phpt new file mode 100644 index 000000000000..b620e48c313d --- /dev/null +++ b/ext/snmp/tests/gh25137.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-25137 (SNMP accepts malformed remote ports) +--EXTENSIONS-- +snmp +--FILE-- +getMessage(), PHP_EOL; + } +} + +foreach (['127.0.0.1:0', '127.0.0.1:65535'] as $hostname) { + var_dump(new SNMP(SNMP::VERSION_1, $hostname, 'public') instanceof SNMP); +} + +try { + snmpget('127.0.0.1:not-a-port', 'public', '1.3.6.1.2.1.1.1.0'); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +ValueError: SNMP::__construct(): Argument #2 ($hostname) remote port must be between 0 and 65535 +ValueError: SNMP::__construct(): Argument #2 ($hostname) remote port must be between 0 and 65535 +ValueError: SNMP::__construct(): Argument #2 ($hostname) remote port must be between 0 and 65535 +ValueError: SNMP::__construct(): Argument #2 ($hostname) remote port must be between 0 and 65535 +ValueError: SNMP::__construct(): Argument #2 ($hostname) remote port must be between 0 and 65535 +ValueError: SNMP::__construct(): Argument #2 ($hostname) remote port must be between 0 and 65535 +ValueError: SNMP::__construct(): Argument #2 ($hostname) remote port must be between 0 and 65535 +ValueError: SNMP::__construct(): Argument #2 ($hostname) remote port must be between 0 and 65535 +bool(true) +bool(true) +ValueError: snmpget(): Argument #1 ($hostname) remote port must be between 0 and 65535