Skip to content

Commit 41458c6

Browse files
devnexendevnexen
committed
Fix GH-21336: undefined behavior in snmp setSecurity.
close GH-21337
1 parent 94853b6 commit 41458c6

File tree

3 files changed

+76
-8
lines changed

3 files changed

+76
-8
lines changed

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ PHP NEWS
99
. Fixed re-entrancy issue on php_pcre_match_impl, php_pcre_replace_impl,
1010
php_pcre_split_impl, and php_pcre_grep_impl. (David Carlier)
1111

12+
- SNMP:
13+
. Fixed bug GH-21336 (SNMP::setSecurity() undefined behavior with
14+
NULL arguments). (David Carlier)
15+
1216
- Standard:
1317
. Fixed bug GH-20906 (Assertion failure when messing up output buffers).
1418
(ndossche)

‎ext/snmp/snmp.c‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,7 @@ static bool netsnmp_session_set_sec_level(struct snmp_session *s, zend_string *l
959959
/* }}} */
960960

961961
/* {{{ Set the authentication protocol in the snmpv3 session */
962-
static bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_string *prot)
962+
static ZEND_ATTRIBUTE_NONNULL bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_string *prot)
963963
{
964964
#ifndef DISABLE_MD5
965965
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
10111011
/* }}} */
10121012

10131013
/* {{{ Set the security protocol in the snmpv3 session */
1014-
static bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string *prot)
1014+
static ZEND_ATTRIBUTE_NONNULL bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string *prot)
10151015
{
10161016
#ifndef NETSNMP_DISABLE_DES
10171017
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
10481048
/* }}} */
10491049

10501050
/* {{{ Make key from pass phrase in the snmpv3 session */
1051-
static bool netsnmp_session_gen_auth_key(struct snmp_session *s, zend_string *pass)
1051+
static ZEND_ATTRIBUTE_NONNULL bool netsnmp_session_gen_auth_key(struct snmp_session *s, zend_string *pass)
10521052
{
10531053
int snmp_errno;
1054+
10541055
s->securityAuthKeyLen = USM_AUTH_KU_LEN;
10551056
if ((snmp_errno = generate_Ku(s->securityAuthProto, s->securityAuthProtoLen,
10561057
(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
10631064
/* }}} */
10641065

10651066
/* {{{ Make key from pass phrase in the snmpv3 session */
1066-
static bool netsnmp_session_gen_sec_key(struct snmp_session *s, zend_string *pass)
1067+
static ZEND_ATTRIBUTE_NONNULL bool netsnmp_session_gen_sec_key(struct snmp_session *s, zend_string *pass)
10671068
{
10681069
int snmp_errno;
10691070

@@ -1102,9 +1103,10 @@ static bool netsnmp_session_set_contextEngineID(struct snmp_session *s, zend_str
11021103
/* }}} */
11031104

11041105
/* {{{ Set all snmpv3-related security options */
1105-
static bool netsnmp_session_set_security(struct snmp_session *session, zend_string *sec_level,
1106+
static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool netsnmp_session_set_security(struct snmp_session *session, zend_string *sec_level,
11061107
zend_string *auth_protocol, zend_string *auth_passphrase, zend_string *priv_protocol,
1107-
zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID)
1108+
zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID,
1109+
uint32_t auth_protocol_argnum)
11081110
{
11091111

11101112
/* Setting the security level. */
@@ -1115,25 +1117,46 @@ static bool netsnmp_session_set_security(struct snmp_session *session, zend_stri
11151117

11161118
if (session->securityLevel == SNMP_SEC_LEVEL_AUTHNOPRIV || session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) {
11171119

1120+
if (!auth_protocol) {
1121+
zend_argument_value_error(auth_protocol_argnum, "cannot be null when security level is \"authNoPriv\" or \"authPriv\"");
1122+
return false;
1123+
}
1124+
11181125
/* Setting the authentication protocol. */
11191126
if (!netsnmp_session_set_auth_protocol(session, auth_protocol)) {
11201127
/* ValueError already generated, just bail out */
11211128
return false;
11221129
}
11231130

1131+
if (!auth_passphrase) {
1132+
zend_argument_value_error(auth_protocol_argnum + 1, "cannot be null when security level is \"authNoPriv\" or \"authPriv\"");
1133+
return false;
1134+
}
1135+
11241136
/* Setting the authentication passphrase. */
11251137
if (!netsnmp_session_gen_auth_key(session, auth_passphrase)) {
11261138
/* Warning message sent already, just bail out */
11271139
return false;
11281140
}
11291141

11301142
if (session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) {
1143+
1144+
if (!priv_protocol) {
1145+
zend_argument_value_error(auth_protocol_argnum + 2, "cannot be null when security level is \"authPriv\"");
1146+
return false;
1147+
}
1148+
11311149
/* Setting the security protocol. */
11321150
if (!netsnmp_session_set_sec_protocol(session, priv_protocol)) {
11331151
/* ValueError already generated, just bail out */
11341152
return false;
11351153
}
11361154

1155+
if (!priv_passphrase) {
1156+
zend_argument_value_error(auth_protocol_argnum + 3, "cannot be null when security level is \"authPriv\"");
1157+
return false;
1158+
}
1159+
11371160
/* Setting the security protocol passphrase. */
11381161
if (!netsnmp_session_gen_sec_key(session, priv_passphrase)) {
11391162
/* Warning message sent already, just bail out */
@@ -1294,7 +1317,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12941317
netsnmp_session_free(&session);
12951318
RETURN_FALSE;
12961319
}
1297-
if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
1320+
if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL, 4)) {
12981321
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
12991322
netsnmp_session_free(&session);
13001323
/* Warning message sent already, just bail out */
@@ -1669,7 +1692,7 @@ PHP_METHOD(SNMP, setSecurity)
16691692
RETURN_THROWS();
16701693
}
16711694

1672-
if (!netsnmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7)) {
1695+
if (!netsnmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7, 2)) {
16731696
/* Warning message sent already, just bail out */
16741697
RETURN_FALSE;
16751698
}

‎ext/snmp/tests/gh21336.phpt‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
GH-21336 (undefined behavior in snmp - NULL pointer dereference in setSecurity)
3+
--EXTENSIONS--
4+
snmp
5+
--FILE--
6+
<?php
7+
$session = new SNMP(SNMP::VERSION_3, 'localhost', 'user');
8+
9+
// auth protocol NULL
10+
try {
11+
$session->setSecurity('authPriv');
12+
} catch (ValueError $e) {
13+
echo $e->getMessage() . PHP_EOL;
14+
}
15+
16+
// auth passphrase NULL
17+
try {
18+
$session->setSecurity('authNoPriv', 'MD5');
19+
} catch (ValueError $e) {
20+
echo $e->getMessage() . PHP_EOL;
21+
}
22+
23+
// priv protocol NULL
24+
try {
25+
$session->setSecurity('authPriv', 'MD5', 'test12345');
26+
} catch (ValueError $e) {
27+
echo $e->getMessage() . PHP_EOL;
28+
}
29+
30+
// priv passphrase NULL
31+
try {
32+
$session->setSecurity('authPriv', 'MD5', 'test12345', 'AES');
33+
} catch (ValueError $e) {
34+
echo $e->getMessage() . PHP_EOL;
35+
}
36+
?>
37+
--EXPECT--
38+
SNMP::setSecurity(): Argument #2 ($authProtocol) cannot be null when security level is "authNoPriv" or "authPriv"
39+
SNMP::setSecurity(): Argument #3 ($authPassphrase) cannot be null when security level is "authNoPriv" or "authPriv"
40+
SNMP::setSecurity(): Argument #4 ($privacyProtocol) cannot be null when security level is "authPriv"
41+
SNMP::setSecurity(): Argument #5 ($privacyPassphrase) cannot be null when security level is "authPriv"

0 commit comments

Comments
 (0)