From 90862fe71add6b7e29f1827578690ae4ded1a35c Mon Sep 17 00:00:00 2001 From: David Carlier Date: Wed, 4 Mar 2026 05:37:42 +0000 Subject: [PATCH 1/3] Fix GH-21336: undefined behavior in snmp setSecurity. --- ext/snmp/snmp.c | 19 +++++++++++++++++ ext/snmp/tests/gh21336.phpt | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 ext/snmp/tests/gh21336.phpt diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c index 0654a97dfc861..07f4f89fca612 100644 --- a/ext/snmp/snmp.c +++ b/ext/snmp/snmp.c @@ -961,6 +961,10 @@ static bool netsnmp_session_set_sec_level(struct snmp_session *s, zend_string *l /* {{{ Set the authentication protocol in the snmpv3 session */ static bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_string *prot) { + if (!prot) { + zend_value_error("Authentication protocol can't be NULL"); + return false; + } #ifndef DISABLE_MD5 if (zend_string_equals_literal_ci(prot, "MD5")) { s->securityAuthProto = usmHMACMD5AuthProtocol; @@ -1013,6 +1017,10 @@ static bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_strin /* {{{ Set the security protocol in the snmpv3 session */ static bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string *prot) { + if (!prot) { + zend_value_error("Security protocol can't be NULL"); + return false; + } #ifndef NETSNMP_DISABLE_DES if (zend_string_equals_literal_ci(prot, "DES")) { s->securityPrivProto = usmDESPrivProtocol; @@ -1051,6 +1059,12 @@ static bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string static bool netsnmp_session_gen_auth_key(struct snmp_session *s, zend_string *pass) { int snmp_errno; + + if (!pass) { + zend_value_error("Authentication key can't be NULL"); + return false; + } + s->securityAuthKeyLen = USM_AUTH_KU_LEN; if ((snmp_errno = generate_Ku(s->securityAuthProto, s->securityAuthProtoLen, (uint8_t *) ZSTR_VAL(pass), ZSTR_LEN(pass), @@ -1067,6 +1081,11 @@ static bool netsnmp_session_gen_sec_key(struct snmp_session *s, zend_string *pas { int snmp_errno; + if (!pass) { + zend_value_error("Security key can't be NULL"); + return false; + } + s->securityPrivKeyLen = USM_PRIV_KU_LEN; if ((snmp_errno = generate_Ku(s->securityAuthProto, s->securityAuthProtoLen, (uint8_t *)ZSTR_VAL(pass), ZSTR_LEN(pass), diff --git a/ext/snmp/tests/gh21336.phpt b/ext/snmp/tests/gh21336.phpt new file mode 100644 index 0000000000000..944c1e9f7f009 --- /dev/null +++ b/ext/snmp/tests/gh21336.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-21336 (undefined behavior in snmp - NULL pointer dereference in setSecurity) +--EXTENSIONS-- +snmp +--FILE-- +setSecurity('authPriv'); +} catch (ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} + +// auth passphrase NULL +try { + $session->setSecurity('authNoPriv', 'MD5'); +} catch (ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} + +// priv protocol NULL +try { + $session->setSecurity('authPriv', 'MD5', 'test12345'); +} catch (ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} + +// priv passphrase NULL +try { + $session->setSecurity('authPriv', 'MD5', 'test12345', 'AES'); +} catch (ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +?> +--EXPECT-- +Authentication protocol can't be NULL +Authentication key can't be NULL +Security protocol can't be NULL +Security key can't be NULL From 097bccf106caa9892fc64851e7b2ae9fcf9dd41e Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 5 Mar 2026 12:48:41 +0000 Subject: [PATCH 2/3] Fix GH-21336: address code review for snmp setSecurity. --- ext/snmp/snmp.c | 49 ++++++++++++++++++++----------------- ext/snmp/tests/gh21336.phpt | 8 +++--- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c index 07f4f89fca612..e9bbca09bdcb3 100644 --- a/ext/snmp/snmp.c +++ b/ext/snmp/snmp.c @@ -959,12 +959,8 @@ static bool netsnmp_session_set_sec_level(struct snmp_session *s, zend_string *l /* }}} */ /* {{{ Set the authentication protocol in the snmpv3 session */ -static bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_string *prot) +static ZEND_ATTRIBUTE_NONNULL bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_string *prot) { - if (!prot) { - zend_value_error("Authentication protocol can't be NULL"); - return false; - } #ifndef DISABLE_MD5 if (zend_string_equals_literal_ci(prot, "MD5")) { s->securityAuthProto = usmHMACMD5AuthProtocol; @@ -1015,12 +1011,8 @@ static bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_strin /* }}} */ /* {{{ Set the security protocol in the snmpv3 session */ -static bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string *prot) +static ZEND_ATTRIBUTE_NONNULL bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string *prot) { - if (!prot) { - zend_value_error("Security protocol can't be NULL"); - return false; - } #ifndef NETSNMP_DISABLE_DES if (zend_string_equals_literal_ci(prot, "DES")) { s->securityPrivProto = usmDESPrivProtocol; @@ -1056,15 +1048,10 @@ static bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string /* }}} */ /* {{{ Make key from pass phrase in the snmpv3 session */ -static bool netsnmp_session_gen_auth_key(struct snmp_session *s, zend_string *pass) +static ZEND_ATTRIBUTE_NONNULL bool netsnmp_session_gen_auth_key(struct snmp_session *s, zend_string *pass) { int snmp_errno; - if (!pass) { - zend_value_error("Authentication key can't be NULL"); - return false; - } - s->securityAuthKeyLen = USM_AUTH_KU_LEN; if ((snmp_errno = generate_Ku(s->securityAuthProto, s->securityAuthProtoLen, (uint8_t *) ZSTR_VAL(pass), ZSTR_LEN(pass), @@ -1077,15 +1064,10 @@ static bool netsnmp_session_gen_auth_key(struct snmp_session *s, zend_string *pa /* }}} */ /* {{{ Make key from pass phrase in the snmpv3 session */ -static bool netsnmp_session_gen_sec_key(struct snmp_session *s, zend_string *pass) +static ZEND_ATTRIBUTE_NONNULL bool netsnmp_session_gen_sec_key(struct snmp_session *s, zend_string *pass) { int snmp_errno; - if (!pass) { - zend_value_error("Security key can't be NULL"); - return false; - } - s->securityPrivKeyLen = USM_PRIV_KU_LEN; if ((snmp_errno = generate_Ku(s->securityAuthProto, s->securityAuthProtoLen, (uint8_t *)ZSTR_VAL(pass), ZSTR_LEN(pass), @@ -1121,7 +1103,7 @@ static bool netsnmp_session_set_contextEngineID(struct snmp_session *s, zend_str /* }}} */ /* {{{ Set all snmpv3-related security options */ -static bool netsnmp_session_set_security(struct snmp_session *session, zend_string *sec_level, +static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool netsnmp_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) { @@ -1134,12 +1116,22 @@ static bool netsnmp_session_set_security(struct snmp_session *session, zend_stri if (session->securityLevel == SNMP_SEC_LEVEL_AUTHNOPRIV || session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) { + if (!auth_protocol) { + zend_value_error("Authentication protocol is required for security level \"authNoPriv\" or \"authPriv\""); + return false; + } + /* Setting the authentication protocol. */ if (!netsnmp_session_set_auth_protocol(session, auth_protocol)) { /* ValueError already generated, just bail out */ return false; } + if (!auth_passphrase) { + zend_value_error("Authentication passphrase is required for security level \"authNoPriv\" or \"authPriv\""); + return false; + } + /* Setting the authentication passphrase. */ if (!netsnmp_session_gen_auth_key(session, auth_passphrase)) { /* Warning message sent already, just bail out */ @@ -1147,12 +1139,23 @@ static bool netsnmp_session_set_security(struct snmp_session *session, zend_stri } if (session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) { + + if (!priv_protocol) { + zend_value_error("Privacy protocol is required for security level \"authPriv\""); + return false; + } + /* Setting the security protocol. */ if (!netsnmp_session_set_sec_protocol(session, priv_protocol)) { /* ValueError already generated, just bail out */ return false; } + if (!priv_passphrase) { + zend_value_error("Privacy passphrase is required for security level \"authPriv\""); + return false; + } + /* Setting the security protocol passphrase. */ if (!netsnmp_session_gen_sec_key(session, priv_passphrase)) { /* Warning message sent already, just bail out */ diff --git a/ext/snmp/tests/gh21336.phpt b/ext/snmp/tests/gh21336.phpt index 944c1e9f7f009..091813f340bae 100644 --- a/ext/snmp/tests/gh21336.phpt +++ b/ext/snmp/tests/gh21336.phpt @@ -35,7 +35,7 @@ try { } ?> --EXPECT-- -Authentication protocol can't be NULL -Authentication key can't be NULL -Security protocol can't be NULL -Security key can't be NULL +Authentication protocol is required for security level "authNoPriv" or "authPriv" +Authentication passphrase is required for security level "authNoPriv" or "authPriv" +Privacy protocol is required for security level "authPriv" +Privacy passphrase is required for security level "authPriv" From 293b85ed4def4bc475a81a1940da32aba901d66a Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 5 Mar 2026 13:24:22 +0000 Subject: [PATCH 3/3] Fix GH-21336: use zend_argument_value_error in snmp setSecurity. --- ext/snmp/snmp.c | 15 ++++++++------- ext/snmp/tests/gh21336.phpt | 8 ++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c index e9bbca09bdcb3..22eb6525f8e0f 100644 --- a/ext/snmp/snmp.c +++ b/ext/snmp/snmp.c @@ -1105,7 +1105,8 @@ static bool netsnmp_session_set_contextEngineID(struct snmp_session *s, zend_str /* {{{ Set all snmpv3-related security options */ static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool netsnmp_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) + zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID, + uint32_t auth_protocol_argnum) { /* Setting the security level. */ @@ -1117,7 +1118,7 @@ static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool netsnmp_session_set_security(struct s if (session->securityLevel == SNMP_SEC_LEVEL_AUTHNOPRIV || session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) { if (!auth_protocol) { - zend_value_error("Authentication protocol is required for security level \"authNoPriv\" or \"authPriv\""); + zend_argument_value_error(auth_protocol_argnum, "cannot be null when security level is \"authNoPriv\" or \"authPriv\""); return false; } @@ -1128,7 +1129,7 @@ static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool netsnmp_session_set_security(struct s } if (!auth_passphrase) { - zend_value_error("Authentication passphrase is required for security level \"authNoPriv\" or \"authPriv\""); + zend_argument_value_error(auth_protocol_argnum + 1, "cannot be null when security level is \"authNoPriv\" or \"authPriv\""); return false; } @@ -1141,7 +1142,7 @@ static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool netsnmp_session_set_security(struct s if (session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) { if (!priv_protocol) { - zend_value_error("Privacy protocol is required for security level \"authPriv\""); + zend_argument_value_error(auth_protocol_argnum + 2, "cannot be null when security level is \"authPriv\""); return false; } @@ -1152,7 +1153,7 @@ static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool netsnmp_session_set_security(struct s } if (!priv_passphrase) { - zend_value_error("Privacy passphrase is required for security level \"authPriv\""); + zend_argument_value_error(auth_protocol_argnum + 3, "cannot be null when security level is \"authPriv\""); return false; } @@ -1316,7 +1317,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version) netsnmp_session_free(&session); RETURN_FALSE; } - if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) { + if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL, 4)) { php_free_objid_query(&objid_query, oid_ht, value_ht, st); netsnmp_session_free(&session); /* Warning message sent already, just bail out */ @@ -1691,7 +1692,7 @@ PHP_METHOD(SNMP, setSecurity) RETURN_THROWS(); } - if (!netsnmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7)) { + if (!netsnmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7, 2)) { /* Warning message sent already, just bail out */ RETURN_FALSE; } diff --git a/ext/snmp/tests/gh21336.phpt b/ext/snmp/tests/gh21336.phpt index 091813f340bae..a4070136e550e 100644 --- a/ext/snmp/tests/gh21336.phpt +++ b/ext/snmp/tests/gh21336.phpt @@ -35,7 +35,7 @@ try { } ?> --EXPECT-- -Authentication protocol is required for security level "authNoPriv" or "authPriv" -Authentication passphrase is required for security level "authNoPriv" or "authPriv" -Privacy protocol is required for security level "authPriv" -Privacy passphrase is required for security level "authPriv" +SNMP::setSecurity(): Argument #2 ($authProtocol) cannot be null when security level is "authNoPriv" or "authPriv" +SNMP::setSecurity(): Argument #3 ($authPassphrase) cannot be null when security level is "authNoPriv" or "authPriv" +SNMP::setSecurity(): Argument #4 ($privacyProtocol) cannot be null when security level is "authPriv" +SNMP::setSecurity(): Argument #5 ($privacyPassphrase) cannot be null when security level is "authPriv"