Skip to content

Commit 81458f5

Browse files
devnexendevnexen
committed
ext/snmp: various internals rewrite.
close phpGH-17368
1 parent bd23d3a commit 81458f5

File tree

2 files changed

+154
-27
lines changed

2 files changed

+154
-27
lines changed

‎ext/snmp/snmp.c‎

Lines changed: 87 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -830,30 +830,69 @@ static bool php_snmp_parse_oid(
830830
/* }}} */
831831

832832
/* {{{ netsnmp_session_init
833-
allocates memory for session and session->peername, caller should free it manually using netsnmp_session_free() and efree()
833+
allocates memory for session and session->peername, caller should free it manually using session_free() and efree()
834834
*/
835-
static bool netsnmp_session_init(php_snmp_session **session_p, int version, zend_string *hostname, zend_string *community, int timeout, int retries)
835+
static bool netsnmp_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)
836836
{
837837
php_snmp_session *session;
838838
char *pptr, *host_ptr;
839839
bool force_ipv6 = false;
840840
int n;
841841
struct sockaddr **psal;
842842
struct sockaddr **res;
843+
844+
*session_p = 0;
845+
846+
ZEND_ASSERT(hostname != NULL);
847+
ZEND_ASSERT(community != NULL);
848+
849+
if (zend_str_has_nul_byte(hostname)) {
850+
zend_argument_value_error(2, "must not contain any null bytes");
851+
return false;
852+
}
853+
854+
if (ZSTR_LEN(hostname) >= MAX_NAME_LEN) {
855+
zend_argument_value_error(2, "length must be lower than %d", MAX_NAME_LEN);
856+
return false;
857+
}
858+
859+
if (zend_str_has_nul_byte(community)) {
860+
zend_argument_value_error(3, "must not contain any null bytes");
861+
return false;
862+
}
863+
864+
if (ZSTR_LEN(community) == 0) {
865+
zend_argument_value_error(3, "cannot be empty");
866+
return false;
867+
}
868+
869+
if (timeout_argument_offset != -1) {
870+
if (timeout < -1 || timeout > LONG_MAX) {
871+
zend_argument_value_error(timeout_argument_offset, "must be between -1 and %ld", LONG_MAX);
872+
return false;
873+
}
874+
875+
if (retries < -1 || retries > INT_MAX) {
876+
zend_argument_value_error(timeout_argument_offset, "must be between -1 and %d", INT_MAX);
877+
return false;
878+
}
879+
}
880+
843881
// TODO: Do not strip and re-add the port in peername?
844-
unsigned remote_port = SNMP_PORT;
882+
unsigned short remote_port = SNMP_PORT;
883+
int tmp_port;
845884

846885
*session_p = (php_snmp_session *)emalloc(sizeof(php_snmp_session));
847886
session = *session_p;
848887
memset(session, 0, sizeof(php_snmp_session));
849888

850889
snmp_sess_init(session);
851890

852-
session->version = version;
891+
session->version = (long)version;
853892

854893
session->peername = emalloc(MAX_NAME_LEN);
855894
/* we copy original hostname for further processing */
856-
strlcpy(session->peername, ZSTR_VAL(hostname), MAX_NAME_LEN);
895+
memcpy(session->peername, ZSTR_VAL(hostname), ZSTR_LEN(hostname) + 1);
857896
host_ptr = session->peername;
858897

859898
/* Reading the hostname and its optional non-default port number */
@@ -862,7 +901,13 @@ static bool netsnmp_session_init(php_snmp_session **session_p, int version, zend
862901
host_ptr++;
863902
if ((pptr = strchr(host_ptr, ']'))) {
864903
if (pptr[1] == ':') {
865-
remote_port = atoi(pptr + 2);
904+
char *pport = pptr + 2;
905+
tmp_port = atoi(pport);
906+
if (tmp_port < 0 || tmp_port > USHRT_MAX) {
907+
zend_value_error("remote port must be between 0 and %u", USHRT_MAX);
908+
return false;
909+
}
910+
remote_port = (unsigned short)tmp_port;
866911
}
867912
*pptr = '\0';
868913
} else {
@@ -871,7 +916,13 @@ static bool netsnmp_session_init(php_snmp_session **session_p, int version, zend
871916
}
872917
} else { /* IPv4 address */
873918
if ((pptr = strchr(host_ptr, ':'))) {
874-
remote_port = atoi(pptr + 1);
919+
char *pport = pptr + 1;
920+
tmp_port = atoi(pport);
921+
if (tmp_port < 0 || tmp_port > USHRT_MAX) {
922+
zend_value_error("remote port must be between 0 and %u", USHRT_MAX);
923+
return false;
924+
}
925+
remote_port = (unsigned short)tmp_port;
875926
*pptr = '\0';
876927
}
877928
}
@@ -920,7 +971,7 @@ static bool netsnmp_session_init(php_snmp_session **session_p, int version, zend
920971
if (remote_port != SNMP_PORT) {
921972
size_t peername_length = strlen(session->peername);
922973
pptr = session->peername + peername_length;
923-
snprintf(pptr, MAX_NAME_LEN - peername_length, ":%d", remote_port);
974+
snprintf(pptr, MAX_NAME_LEN - peername_length, ":%u", remote_port);
924975
}
925976

926977
php_network_freeaddresses(psal);
@@ -935,14 +986,14 @@ static bool netsnmp_session_init(php_snmp_session **session_p, int version, zend
935986
session->community_len = ZSTR_LEN(community);
936987
}
937988

938-
session->retries = retries;
939-
session->timeout = timeout;
989+
session->retries = (int)retries;
990+
session->timeout = (long)timeout;
940991
return true;
941992
}
942993
/* }}} */
943994

944995
/* {{{ Set the security level in the snmpv3 session */
945-
static bool netsnmp_session_set_sec_level(struct snmp_session *s, zend_string *level)
996+
static bool snmp_session_set_sec_level(struct snmp_session *s, zend_string *level)
946997
{
947998
if (zend_string_equals_literal_ci(level, "noAuthNoPriv") || zend_string_equals_literal_ci(level, "nanp")) {
948999
s->securityLevel = SNMP_SEC_LEVEL_NOAUTH;
@@ -959,7 +1010,7 @@ static bool netsnmp_session_set_sec_level(struct snmp_session *s, zend_string *l
9591010
/* }}} */
9601011

9611012
/* {{{ Set the authentication protocol in the snmpv3 session */
962-
static bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_string *prot)
1013+
static bool snmp_session_set_auth_protocol(struct snmp_session *s, zend_string *prot)
9631014
{
9641015
#ifndef DISABLE_MD5
9651016
if (zend_string_equals_literal_ci(prot, "MD5")) {
@@ -1011,7 +1062,7 @@ static bool netsnmp_session_set_auth_protocol(struct snmp_session *s, zend_strin
10111062
/* }}} */
10121063

10131064
/* {{{ Set the security protocol in the snmpv3 session */
1014-
static bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string *prot)
1065+
static bool snmp_session_set_sec_protocol(struct snmp_session *s, zend_string *prot)
10151066
{
10161067
#ifndef NETSNMP_DISABLE_DES
10171068
if (zend_string_equals_literal_ci(prot, "DES")) {
@@ -1048,7 +1099,7 @@ static bool netsnmp_session_set_sec_protocol(struct snmp_session *s, zend_string
10481099
/* }}} */
10491100

10501101
/* {{{ Make key from pass phrase in the snmpv3 session */
1051-
static bool netsnmp_session_gen_auth_key(struct snmp_session *s, zend_string *pass)
1102+
static bool snmp_session_gen_auth_key(struct snmp_session *s, zend_string *pass)
10521103
{
10531104
int snmp_errno;
10541105
s->securityAuthKeyLen = USM_AUTH_KU_LEN;
@@ -1063,7 +1114,7 @@ static bool netsnmp_session_gen_auth_key(struct snmp_session *s, zend_string *pa
10631114
/* }}} */
10641115

10651116
/* {{{ Make key from pass phrase in the snmpv3 session */
1066-
static bool netsnmp_session_gen_sec_key(struct snmp_session *s, zend_string *pass)
1117+
static bool snmp_session_gen_sec_key(struct snmp_session *s, zend_string *pass)
10671118
{
10681119
int snmp_errno;
10691120

@@ -1079,7 +1130,7 @@ static bool netsnmp_session_gen_sec_key(struct snmp_session *s, zend_string *pas
10791130
/* }}} */
10801131

10811132
/* {{{ Set context Engine Id in the snmpv3 session */
1082-
static bool netsnmp_session_set_contextEngineID(struct snmp_session *s, zend_string * contextEngineID)
1133+
static bool snmp_session_set_contextEngineID(struct snmp_session *s, zend_string * contextEngineID)
10831134
{
10841135
size_t ebuf_len = 32, eout_len = 0;
10851136
uint8_t *ebuf = (uint8_t *) emalloc(ebuf_len);
@@ -1102,40 +1153,40 @@ static bool netsnmp_session_set_contextEngineID(struct snmp_session *s, zend_str
11021153
/* }}} */
11031154

11041155
/* {{{ Set all snmpv3-related security options */
1105-
static bool netsnmp_session_set_security(struct snmp_session *session, zend_string *sec_level,
1156+
static bool snmp_session_set_security(struct snmp_session *session, zend_string *sec_level,
11061157
zend_string *auth_protocol, zend_string *auth_passphrase, zend_string *priv_protocol,
11071158
zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID)
11081159
{
11091160

11101161
/* Setting the security level. */
1111-
if (!netsnmp_session_set_sec_level(session, sec_level)) {
1162+
if (!snmp_session_set_sec_level(session, sec_level)) {
11121163
/* ValueError already generated, just bail out */
11131164
return false;
11141165
}
11151166

11161167
if (session->securityLevel == SNMP_SEC_LEVEL_AUTHNOPRIV || session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) {
11171168

11181169
/* Setting the authentication protocol. */
1119-
if (!netsnmp_session_set_auth_protocol(session, auth_protocol)) {
1170+
if (!snmp_session_set_auth_protocol(session, auth_protocol)) {
11201171
/* ValueError already generated, just bail out */
11211172
return false;
11221173
}
11231174

11241175
/* Setting the authentication passphrase. */
1125-
if (!netsnmp_session_gen_auth_key(session, auth_passphrase)) {
1176+
if (!snmp_session_gen_auth_key(session, auth_passphrase)) {
11261177
/* Warning message sent already, just bail out */
11271178
return false;
11281179
}
11291180

11301181
if (session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) {
11311182
/* Setting the security protocol. */
1132-
if (!netsnmp_session_set_sec_protocol(session, priv_protocol)) {
1183+
if (!snmp_session_set_sec_protocol(session, priv_protocol)) {
11331184
/* ValueError already generated, just bail out */
11341185
return false;
11351186
}
11361187

11371188
/* Setting the security protocol passphrase. */
1138-
if (!netsnmp_session_gen_sec_key(session, priv_passphrase)) {
1189+
if (!snmp_session_gen_sec_key(session, priv_passphrase)) {
11391190
/* Warning message sent already, just bail out */
11401191
return false;
11411192
}
@@ -1149,7 +1200,7 @@ static bool netsnmp_session_set_security(struct snmp_session *session, zend_stri
11491200
}
11501201

11511202
/* Setting contextEngineIS if specified */
1152-
if (contextEngineID && ZSTR_LEN(contextEngineID) && !netsnmp_session_set_contextEngineID(session, contextEngineID)) {
1203+
if (contextEngineID && ZSTR_LEN(contextEngineID) && !snmp_session_set_contextEngineID(session, contextEngineID)) {
11531204
/* Warning message sent already, just bail out */
11541205
return false;
11551206
}
@@ -1176,6 +1227,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
11761227
struct objid_query objid_query;
11771228
php_snmp_session *session;
11781229
int session_less_mode = (getThis() == NULL);
1230+
int timeout_argument_offset = -1;
11791231
php_snmp_object *snmp_object;
11801232
php_snmp_object glob_snmp_object;
11811233

@@ -1202,6 +1254,8 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12021254
Z_PARAM_LONG(timeout)
12031255
Z_PARAM_LONG(retries)
12041256
ZEND_PARSE_PARAMETERS_END();
1257+
1258+
timeout_argument_offset = 10;
12051259
} else {
12061260
/* SNMP_CMD_GET
12071261
* SNMP_CMD_GETNEXT
@@ -1220,6 +1274,8 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12201274
Z_PARAM_LONG(timeout)
12211275
Z_PARAM_LONG(retries)
12221276
ZEND_PARSE_PARAMETERS_END();
1277+
1278+
timeout_argument_offset = 9;
12231279
}
12241280
} else {
12251281
if (st & SNMP_CMD_SET) {
@@ -1233,6 +1289,8 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12331289
Z_PARAM_LONG(timeout)
12341290
Z_PARAM_LONG(retries)
12351291
ZEND_PARSE_PARAMETERS_END();
1292+
1293+
timeout_argument_offset = 6;
12361294
} else {
12371295
/* SNMP_CMD_GET
12381296
* SNMP_CMD_GETNEXT
@@ -1246,6 +1304,8 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12461304
Z_PARAM_LONG(timeout)
12471305
Z_PARAM_LONG(retries)
12481306
ZEND_PARSE_PARAMETERS_END();
1307+
1308+
timeout_argument_offset = 4;
12491309
}
12501310
}
12511311
} else {
@@ -1289,12 +1349,12 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12891349
}
12901350

12911351
if (session_less_mode) {
1292-
if (!netsnmp_session_init(&session, version, a1, a2, timeout, retries)) {
1352+
if (!netsnmp_session_init(&session, version, a1, a2, timeout, retries, timeout_argument_offset)) {
12931353
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
12941354
netsnmp_session_free(&session);
12951355
RETURN_FALSE;
12961356
}
1297-
if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
1357+
if (version == SNMP_VERSION_3 && !snmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
12981358
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
12991359
netsnmp_session_free(&session);
13001360
/* Warning message sent already, just bail out */
@@ -1593,7 +1653,7 @@ PHP_METHOD(SNMP, __construct)
15931653
netsnmp_session_free(&(snmp_object->session));
15941654
}
15951655

1596-
if (!netsnmp_session_init(&(snmp_object->session), version, a1, a2, timeout, retries)) {
1656+
if (!netsnmp_session_init(&(snmp_object->session), version, a1, a2, timeout, retries, 4)) {
15971657
return;
15981658
}
15991659
snmp_object->max_oids = 0;
@@ -1669,7 +1729,7 @@ PHP_METHOD(SNMP, setSecurity)
16691729
RETURN_THROWS();
16701730
}
16711731

1672-
if (!netsnmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7)) {
1732+
if (!snmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7)) {
16731733
/* Warning message sent already, just bail out */
16741734
RETURN_FALSE;
16751735
}

‎ext/snmp/tests/snmp_session_error.phpt‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
--TEST--
2+
SNMP::__construct checks
3+
--CREDITS--
4+
Boris Lytochkin
5+
--EXTENSIONS--
6+
snmp
7+
--SKIPIF--
8+
<?php
9+
require_once(__DIR__.'/skipif.inc');
10+
?>
11+
--FILE--
12+
<?php
13+
require_once(__DIR__.'/snmp_include.inc');
14+
15+
$longhostname = str_repeat($hostname4, 1_000_000);
16+
try {
17+
new SNMP(SNMP::VERSION_1, "$hostname4:-1", $community, $timeout, $retries);
18+
} catch (\ValueError $e) {
19+
echo $e->getMessage(), PHP_EOL;
20+
}
21+
try {
22+
new SNMP(SNMP::VERSION_1, "$hostname4:65536", $community, $timeout, $retries);
23+
} catch (\ValueError $e) {
24+
echo $e->getMessage(), PHP_EOL;
25+
}
26+
try {
27+
new SNMP(SNMP::VERSION_1, "$longhostname:$port", $community, $timeout, $retries);
28+
} catch (\ValueError $e) {
29+
echo $e->getMessage(), PHP_EOL;
30+
}
31+
try {
32+
new SNMP(SNMP::VERSION_1, "$hostname:$port", $community, PHP_INT_MIN, $retries);
33+
} catch (\ValueError $e) {
34+
echo $e->getMessage(), PHP_EOL;
35+
}
36+
try {
37+
new SNMP(SNMP::VERSION_1, "$hostname:$port", $community, $timeout, PHP_INT_MAX);
38+
} catch (\ValueError $e) {
39+
echo $e->getMessage(), PHP_EOL;
40+
}
41+
try {
42+
new SNMP(SNMP::VERSION_1, "\0$hostname:$port", $community, $timeout, $retries);
43+
} catch (\ValueError $e) {
44+
echo $e->getMessage(), PHP_EOL;
45+
}
46+
try {
47+
new SNMP(SNMP::VERSION_1, "$hostname:$port", "", $timeout, $retries);
48+
} catch (\ValueError $e) {
49+
echo $e->getMessage(), PHP_EOL;
50+
}
51+
try {
52+
new SNMP(SNMP::VERSION_1, "$hostname:$port", "\0$community", $timeout, $retries);
53+
} catch (\ValueError $e) {
54+
echo $e->getMessage(), PHP_EOL;
55+
}
56+
echo "OK";
57+
?>
58+
--EXPECTF--
59+
remote port must be between 0 and 65535
60+
remote port must be between 0 and 65535
61+
SNMP::__construct(): Argument #2 ($hostname) length must be lower than 128
62+
SNMP::__construct(): Argument #4 ($timeout) must be between -1 and %d
63+
SNMP::__construct(): Argument #4 ($timeout) must be between -1 and %d
64+
SNMP::__construct(): Argument #2 ($hostname) must not contain any null bytes
65+
SNMP::__construct(): Argument #3 ($community) cannot be empty
66+
SNMP::__construct(): Argument #3 ($community) must not contain any null bytes
67+
OK

0 commit comments

Comments
 (0)