Skip to content

Fix GH-21336: undefined behavior in snmp setSecurity. #21337

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

Open
devnexen wants to merge 3 commits into
base: PHP-8.4
Choose a base branch
from
+72 −8
Open

Fix GH-21336: undefined behavior in snmp setSecurity. #21337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90862fe
Fix GH-21336: undefined behavior in snmp setSecurity.
devnexen Mar 4, 2026
097bccf
Fix GH-21336: address code review for snmp setSecurity.
devnexen Mar 5, 2026
293b85e
Fix GH-21336: use zend_argument_value_error in snmp setSecurity.
devnexen Mar 5, 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
39 changes: 31 additions & 8 deletions ext/snmp/snmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down Expand Up @@ -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")) {
Expand Down Expand Up @@ -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),
Expand All @@ -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;

Expand Down Expand Up @@ -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. */
Expand All @@ -1115,25 +1117,46 @@ 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;
}
Comment on lines +1120 to +1123
Copy link
Member

@Girgias Girgias Mar 5, 2026

Choose a reason for hiding this comment

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

Can't you provide the argnum for auth_protocol and then we can derive all the other ones?


/* 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 */
return false;
}

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 */
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
}
Expand Down
41 changes: 41 additions & 0 deletions ext/snmp/tests/gh21336.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
GH-21336 (undefined behavior in snmp - NULL pointer dereference in setSecurity)
--EXTENSIONS--
snmp
--FILE--
<?php
$session = new SNMP(SNMP::VERSION_3, 'localhost', 'user');

// auth protocol NULL
try {
$session->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"
Loading