Skip to content

ext/snmp: promote invalid-input warnings to ValueError #21319

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

Merged
Girgias merged 5 commits into from
Apr 6, 2026
Merged

ext/snmp: promote invalid-input warnings to ValueError #21319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b6965c9
ext/snmp: promote invalid-input warnings to ValueError
prateekbhujel Mar 1, 2026
4a448a7
ext/snmp: include argument numbers in ValueError messages
prateekbhujel Apr 6, 2026
c0d7a92
ext/snmp: update empty object_id ValueError expectation
prateekbhujel Apr 6, 2026
69c7b55
ext/snmp: move null-byte checks into ZPP
prateekbhujel Apr 6, 2026
2733d9f
ext/snmp: use P specifier in SNMP::__construct
prateekbhujel Apr 6, 2026
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
94 changes: 50 additions & 44 deletions ext/snmp/snmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,8 @@ static void php_free_objid_query(struct objid_query *objid_query, HashTable* oid
*/
static bool php_snmp_parse_oid(
zval *object, int st, struct objid_query *objid_query, zend_string *oid_str, HashTable *oid_ht,
zend_string *type_str, HashTable *type_ht, zend_string *value_str, HashTable *value_ht
zend_string *type_str, HashTable *type_ht, zend_string *value_str, HashTable *value_ht,
uint32_t oid_argument_offset, uint32_t type_argument_offset, uint32_t value_argument_offset
) {
char *pptr;
uint32_t idx_type = 0, idx_value = 0;
Expand All @@ -682,7 +683,7 @@ static bool php_snmp_parse_oid(
ZEND_ASSERT(type_str && value_str);

if (ZSTR_LEN(type_str) != 1) {
zend_value_error("Type must be a single character");
zend_argument_value_error(type_argument_offset, "must be a single character");
efree(objid_query->vars);
return false;
}
Expand All @@ -693,7 +694,7 @@ static bool php_snmp_parse_oid(
objid_query->count++;
} else if (oid_ht) { /* we got objid array */
if (zend_hash_num_elements(oid_ht) == 0) {
zend_value_error("Array of object IDs must not be empty");
zend_argument_value_error(oid_argument_offset, "must not be empty when passed as an array");
return false;
}
objid_query->vars = (snmpobjarg *)safe_emalloc(sizeof(snmpobjarg), zend_hash_num_elements(oid_ht), 0);
Expand Down Expand Up @@ -738,14 +739,14 @@ static bool php_snmp_parse_oid(
char ptype = *ZSTR_VAL(type);
zend_string_release(type);
if (len != 1) {
zend_value_error("Type must be a single character");
zend_argument_value_error(type_argument_offset, "must be a single character");
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
objid_query->vars[objid_query->count].type = ptype;
idx_type++;
} else {
php_error_docref(NULL, E_WARNING, "'%s': no type set", ZSTR_VAL(tmp));
zend_argument_value_error(type_argument_offset, "must contain a type for object ID '%s'", ZSTR_VAL(tmp));
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
Expand Down Expand Up @@ -780,7 +781,7 @@ static bool php_snmp_parse_oid(
objid_query->vars[objid_query->count].value = ZSTR_VAL(tmp);
idx_value++;
} else {
php_error_docref(NULL, E_WARNING, "'%s': no value set", ZSTR_VAL(tmp));
zend_argument_value_error(value_argument_offset, "must contain a value for object ID '%s'", ZSTR_VAL(tmp));
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
Expand Down Expand Up @@ -827,7 +828,7 @@ static bool php_snmp_parse_oid(
/* {{{ snmp_session_init
allocates memory for session and session->peername, caller should free it manually using snmp_session_free() and efree()
*/
static bool snmp_session_init(php_snmp_session **session_p, int version, zend_string *hostname, zend_string *community, zend_long timeout, zend_long retries, int timeout_argument_offset)
static bool snmp_session_init(php_snmp_session **session_p, int version, zend_string *hostname, zend_string *community, zend_long timeout, zend_long retries, int hostname_argument_offset, int timeout_argument_offset)
{
php_snmp_session *session;
char *pptr, *host_ptr;
Expand All @@ -841,23 +842,13 @@ static bool snmp_session_init(php_snmp_session **session_p, int version, zend_st
ZEND_ASSERT(hostname != NULL);
ZEND_ASSERT(community != NULL);

if (zend_str_has_nul_byte(hostname)) {
zend_argument_value_error(2, "must not contain any null bytes");
return false;
}

if (ZSTR_LEN(hostname) >= MAX_NAME_LEN) {
zend_argument_value_error(2, "length must be lower than %d", MAX_NAME_LEN);
return false;
}

if (zend_str_has_nul_byte(community)) {
zend_argument_value_error(3, "must not contain any null bytes");
zend_argument_value_error(hostname_argument_offset, "length must be lower than %d", MAX_NAME_LEN);
return false;
}

if (ZSTR_LEN(community) == 0) {
zend_argument_must_not_be_empty_error(3);
zend_argument_must_not_be_empty_error(hostname_argument_offset + 1);
return false;
}

Expand Down Expand Up @@ -899,22 +890,22 @@ static bool snmp_session_init(php_snmp_session **session_p, int version, zend_st
char *pport = pptr + 2;
tmp_port = atoi(pport);
if (tmp_port < 0 || tmp_port > USHRT_MAX) {
zend_argument_value_error(2, "remote port must be between 0 and %u", USHRT_MAX);
zend_argument_value_error(hostname_argument_offset, "remote port must be between 0 and %u", USHRT_MAX);
return false;
}
remote_port = (unsigned short)tmp_port;
}
*pptr = '\0';
} else {
php_error_docref(NULL, E_WARNING, "Malformed IPv6 address, closing square bracket missing");
zend_argument_value_error(hostname_argument_offset, "has a malformed IPv6 address, closing square bracket missing");
return false;
}
} else { /* IPv4 address */
if ((pptr = strchr(host_ptr, ':'))) {
char *pport = pptr + 1;
tmp_port = atoi(pport);
if (tmp_port < 0 || tmp_port > USHRT_MAX) {
zend_argument_value_error(2, "remote port must be between 0 and %u", USHRT_MAX);
zend_argument_value_error(hostname_argument_offset, "remote port must be between 0 and %u", USHRT_MAX);
return false;
}
remote_port = (unsigned short)tmp_port;
Expand Down Expand Up @@ -1123,14 +1114,13 @@ static ZEND_ATTRIBUTE_NONNULL bool snmp_session_gen_sec_key(struct snmp_session
/* }}} */

/* {{{ Set context Engine Id in the snmpv3 session */
static bool snmp_session_set_contextEngineID(struct snmp_session *s, zend_string * contextEngineID)
static bool snmp_session_set_contextEngineID(struct snmp_session *s, zend_string * contextEngineID, uint32_t contextEngineID_argument_offset)
{
size_t ebuf_len = 32, eout_len = 0;
uint8_t *ebuf = (uint8_t *) emalloc(ebuf_len);

if (!snmp_hex_to_binary(&ebuf, &ebuf_len, &eout_len, 1, ZSTR_VAL(contextEngineID))) {
// TODO Promote to Error?
php_error_docref(NULL, E_WARNING, "Bad engine ID value '%s'", ZSTR_VAL(contextEngineID));
zend_argument_value_error(contextEngineID_argument_offset, "must be a valid context engine ID");
efree(ebuf);
return false;
}
Expand All @@ -1145,11 +1135,14 @@ static bool snmp_session_set_contextEngineID(struct snmp_session *s, zend_string
}
/* }}} */

/* {{{ Set all snmpv3-related security options */
/* {{{ Set all snmpv3-related security options
* auth_protocol_argnum and contextEngineID_argument_offset are the userland
* argument numbers used for error reporting.
*/
static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool snmp_session_set_security(struct snmp_session *session, zend_string *sec_level,
zend_string *auth_protocol, zend_string *auth_passphrase, zend_string *priv_protocol,
zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID,
uint32_t auth_protocol_argnum)
uint32_t auth_protocol_argnum, uint32_t contextEngineID_argument_offset)
{

/* Setting the security level. */
Expand Down Expand Up @@ -1215,7 +1208,7 @@ static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool snmp_session_set_security(struct snmp
}

/* Setting contextEngineIS if specified */
if (contextEngineID && ZSTR_LEN(contextEngineID) && !snmp_session_set_contextEngineID(session, contextEngineID)) {
if (contextEngineID && ZSTR_LEN(contextEngineID) && !snmp_session_set_contextEngineID(session, contextEngineID, contextEngineID_argument_offset)) {
/* Warning message sent already, just bail out */
return false;
}
Expand Down Expand Up @@ -1243,6 +1236,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
php_snmp_session *session;
int session_less_mode = (getThis() == NULL);
int timeout_argument_offset = -1;
uint32_t oid_argument_offset = 1, type_argument_offset = 0, value_argument_offset = 0;
php_snmp_object *snmp_object;
php_snmp_object glob_snmp_object;

Expand All @@ -1255,8 +1249,8 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
if (version == SNMP_VERSION_3) {
if (st & SNMP_CMD_SET) {
ZEND_PARSE_PARAMETERS_START(10, 12)
Z_PARAM_STR(a1)
Z_PARAM_STR(a2)
Z_PARAM_PATH_STR(a1)
Z_PARAM_PATH_STR(a2)
Z_PARAM_STR(a3)
Z_PARAM_STR(a4)
Z_PARAM_STR(a5)
Expand All @@ -1271,14 +1265,17 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
ZEND_PARSE_PARAMETERS_END();

timeout_argument_offset = 10;
oid_argument_offset = 8;
type_argument_offset = 9;
value_argument_offset = 10;
} else {
/* SNMP_CMD_GET
* SNMP_CMD_GETNEXT
* SNMP_CMD_WALK
*/
ZEND_PARSE_PARAMETERS_START(8, 10)
Z_PARAM_STR(a1)
Z_PARAM_STR(a2)
Z_PARAM_PATH_STR(a1)
Z_PARAM_PATH_STR(a2)
Z_PARAM_STR(a3)
Z_PARAM_STR(a4)
Z_PARAM_STR(a5)
Expand All @@ -1291,12 +1288,13 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
ZEND_PARSE_PARAMETERS_END();

timeout_argument_offset = 9;
oid_argument_offset = 8;
}
} else {
if (st & SNMP_CMD_SET) {
ZEND_PARSE_PARAMETERS_START(5, 7)
Z_PARAM_STR(a1)
Z_PARAM_STR(a2)
Z_PARAM_PATH_STR(a1)
Z_PARAM_PATH_STR(a2)
Z_PARAM_ARRAY_HT_OR_STR(oid_ht, oid_str)
Z_PARAM_ARRAY_HT_OR_STR(type_ht, type_str)
Z_PARAM_ARRAY_HT_OR_STR(value_ht, value_str)
Expand All @@ -1306,21 +1304,25 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
ZEND_PARSE_PARAMETERS_END();

timeout_argument_offset = 6;
oid_argument_offset = 3;
type_argument_offset = 4;
value_argument_offset = 5;
} else {
/* SNMP_CMD_GET
* SNMP_CMD_GETNEXT
* SNMP_CMD_WALK
*/
ZEND_PARSE_PARAMETERS_START(3, 5)
Z_PARAM_STR(a1)
Z_PARAM_STR(a2)
Z_PARAM_PATH_STR(a1)
Z_PARAM_PATH_STR(a2)
Z_PARAM_ARRAY_HT_OR_STR(oid_ht, oid_str)
Z_PARAM_OPTIONAL
Z_PARAM_LONG(timeout)
Z_PARAM_LONG(retries)
ZEND_PARSE_PARAMETERS_END();

timeout_argument_offset = 4;
oid_argument_offset = 3;
}
}
} else {
Expand All @@ -1330,6 +1332,8 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
Z_PARAM_ARRAY_HT_OR_STR(type_ht, type_str)
Z_PARAM_ARRAY_HT_OR_STR(value_ht, value_str)
ZEND_PARSE_PARAMETERS_END();
type_argument_offset = 2;
value_argument_offset = 3;
} else if (st & SNMP_CMD_WALK) {
ZEND_PARSE_PARAMETERS_START(1, 4)
Z_PARAM_ARRAY_HT_OR_STR(oid_ht, oid_str)
Expand Down Expand Up @@ -1359,20 +1363,21 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
}
}

if (!php_snmp_parse_oid(getThis(), st, &objid_query, oid_str, oid_ht, type_str, type_ht, value_str, value_ht)) {
if (!php_snmp_parse_oid(getThis(), st, &objid_query, oid_str, oid_ht, type_str, type_ht, value_str, value_ht,
oid_argument_offset, type_argument_offset, value_argument_offset)) {
RETURN_FALSE;
}

if (session_less_mode) {
if (!snmp_session_init(&session, version, a1, a2, timeout, retries, timeout_argument_offset)) {
if (!snmp_session_init(&session, version, a1, a2, timeout, retries, 1, timeout_argument_offset)) {
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
snmp_session_free(&session);
RETURN_FALSE;
}
if (version == SNMP_VERSION_3 && !snmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL, 4)) {
if (version == SNMP_VERSION_3 && !snmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL, 4, 0)) {
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
snmp_session_free(&session);
/* Warning message sent already, just bail out */
/* An error has already been emitted, just bail out. */
RETURN_FALSE;
}
} else {
Expand Down Expand Up @@ -1645,7 +1650,7 @@ PHP_METHOD(SNMP, __construct)

snmp_object = Z_SNMP_P(object);

if (zend_parse_parameters(ZEND_NUM_ARGS(), "lSS|ll", &version, &a1, &a2, &timeout, &retries) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "lPP|ll", &version, &a1, &a2, &timeout, &retries) == FAILURE) {
RETURN_THROWS();
}

Expand All @@ -1664,7 +1669,7 @@ PHP_METHOD(SNMP, __construct)
snmp_session_free(&(snmp_object->session));
}

if (!snmp_session_init(&(snmp_object->session), version, a1, a2, timeout, retries, 4)) {
if (!snmp_session_init(&(snmp_object->session), version, a1, a2, timeout, retries, 2, 4)) {
return;
}
snmp_object->max_oids = 0;
Expand Down Expand Up @@ -1738,8 +1743,9 @@ PHP_METHOD(SNMP, setSecurity)
RETURN_THROWS();
}

if (!snmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7, 2)) {
/* Warning message sent already, just bail out */
/* authProtocol is argument #2 and contextEngineId is argument #7. */
if (!snmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7, 2, 7)) {
/* An error has already been emitted, just bail out. */
RETURN_FALSE;
}
RETURN_TRUE;
Expand Down
4 changes: 2 additions & 2 deletions ext/snmp/tests/gh16959.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,5 @@ array(4) {
}
Object of class stdClass could not be converted to string
Object of class stdClass could not be converted to string
Type must be a single character
Type must be a single character
snmp2_set(): Argument #4 ($type) must be a single character
snmp2_set(): Argument #4 ($type) must be a single character
10 changes: 6 additions & 4 deletions ext/snmp/tests/ipv6.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ snmp_set_quick_print(false);
snmp_set_valueretrieval(SNMP_VALUE_PLAIN);

var_dump(snmpget($hostname6_port, $community, '.1.3.6.1.2.1.1.1.0'));
var_dump(snmpget('[dead:beef::', $community, '.1.3.6.1.2.1.1.1.0'));
try {
var_dump(snmpget('[dead:beef::', $community, '.1.3.6.1.2.1.1.1.0'));
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
?>
--EXPECTF--
string(%d) "%s"

Warning: snmpget(): Malformed IPv6 address, closing square bracket missing in %s on line %d
bool(false)
snmpget(): Argument #1 ($hostname) has a malformed IPv6 address, closing square bracket missing
10 changes: 6 additions & 4 deletions ext/snmp/tests/snmp-object-setSecurity_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ try {

var_dump($session->setSecurity('authPriv', 'MD5', $auth_pass, 'AES', ''));
var_dump($session->setSecurity('authPriv', 'MD5', $auth_pass, 'AES', 'ty'));
var_dump($session->setSecurity('authPriv', 'MD5', $auth_pass, 'AES', 'test12345', 'context', 'dsa'));
try {
var_dump($session->setSecurity('authPriv', 'MD5', $auth_pass, 'AES', 'test12345', 'context', 'dsa'));
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}

var_dump($session->close());

Expand All @@ -76,7 +80,5 @@ bool(false)

Warning: SNMP::setSecurity(): Error generating a key for privacy pass phrase 'ty': Generic error (The supplied password length is too short.) in %s on line %d
bool(false)

Warning: SNMP::setSecurity(): Bad engine ID value 'dsa' in %s on line %d
bool(false)
SNMP::setSecurity(): Argument #7 ($contextEngineId) must be a valid context engine ID
bool(true)
2 changes: 1 addition & 1 deletion ext/snmp/tests/snmp2_get.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ var_dump(snmp2_get($hostname, $community, array('.1.3.6.1.2.1.1.1.0', '.1.3.6.1.
--EXPECTF--
Checking error handling
Empty OID array
Array of object IDs must not be empty
snmp2_get(): Argument #3 ($object_id) must not be empty when passed as an array
Checking working
Single OID
string(%d) "%s"
Expand Down
Loading
Loading