diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c index 0654a97dfc861..22eb6525f8e0f 100644 --- a/ext/snmp/snmp.c +++ b/ext/snmp/snmp.c @@ -959,7 +959,7 @@ 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) { #ifndef DISABLE_MD5 if (zend_string_equals_literal_ci(prot, "MD5")) { @@ -1011,7 +1011,7 @@ 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) { #ifndef NETSNMP_DISABLE_DES if (zend_string_equals_literal_ci(prot, "DES")) { @@ -1048,9 +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; + s->securityAuthKeyLen = USM_AUTH_KU_LEN; if ((snmp_errno = generate_Ku(s->securityAuthProto, s->securityAuthProtoLen, (uint8_t *) ZSTR_VAL(pass), ZSTR_LEN(pass), @@ -1063,7 +1064,7 @@ 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; @@ -1102,9 +1103,10 @@ 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) + zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID, + uint32_t auth_protocol_argnum) { /* Setting the security level. */ @@ -1115,12 +1117,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_argument_value_error(auth_protocol_argnum, "cannot be null when security level is \"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_argument_value_error(auth_protocol_argnum + 1, "cannot be null when security level is \"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 */ @@ -1128,12 +1140,23 @@ static bool netsnmp_session_set_security(struct snmp_session *session, zend_stri } if (session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) { + + if (!priv_protocol) { + zend_argument_value_error(auth_protocol_argnum + 2, "cannot be null when security level is \"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_argument_value_error(auth_protocol_argnum + 3, "cannot be null when security level is \"authPriv\""); + return false; + } + /* Setting the security protocol passphrase. */ if (!netsnmp_session_gen_sec_key(session, priv_passphrase)) { /* Warning message sent already, just bail out */ @@ -1294,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 */ @@ -1669,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 new file mode 100644 index 0000000000000..a4070136e550e --- /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-- +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"