Skip to content

Fix GH-16959: snmpget modifies the object_id (as array). #16969

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

Closed
wants to merge 1 commit into from
Closed

Fix GH-16959: snmpget modifies the object_id (as array). #16969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
80 changes: 60 additions & 20 deletions ext/snmp/snmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,31 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
}
/* }}} */

static void php_snmp_zend_string_release_from_char_pointer(char *ptr) {
if (ptr) {
zend_string *pptr = (zend_string *)(ptr - XtOffsetOf(zend_string, val));
zend_string_release(pptr);
}
}

static void php_free_objid_query(struct objid_query *objid_query, HashTable* oid_ht, const HashTable *value_ht, int st) {
if (oid_ht) {
uint32_t count = zend_hash_num_elements(oid_ht);

for (uint32_t i = 0; i < count; i ++) {
snmpobjarg *arg = &objid_query->vars[i];
if (!arg->oid) {
break;
}
if (value_ht) {
php_snmp_zend_string_release_from_char_pointer(arg->value);
}
php_snmp_zend_string_release_from_char_pointer(arg->oid);
}
}
efree(objid_query->vars);
}

/* {{{ php_snmp_parse_oid
*
* OID parser (and type, value for SNMP_SET command)
Expand Down Expand Up @@ -674,10 +699,15 @@ static bool php_snmp_parse_oid(
return false;
}
objid_query->vars = (snmpobjarg *)safe_emalloc(sizeof(snmpobjarg), zend_hash_num_elements(oid_ht), 0);
memset(objid_query->vars, 0, sizeof(snmpobjarg) * zend_hash_num_elements(oid_ht));
objid_query->array_output = (st & SNMP_CMD_SET) == 0;
ZEND_HASH_FOREACH_VAL(oid_ht, tmp_oid) {
convert_to_string(tmp_oid);
objid_query->vars[objid_query->count].oid = Z_STRVAL_P(tmp_oid);
zend_string *tmp = zval_try_get_string(tmp_oid);
if (!tmp) {
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
objid_query->vars[objid_query->count].oid = ZSTR_VAL(tmp);
if (st & SNMP_CMD_SET) {
if (type_str) {
pptr = ZSTR_VAL(type_str);
Expand All @@ -701,18 +731,24 @@ static bool php_snmp_parse_oid(
}
}
if (idx_type < type_ht->nNumUsed) {
convert_to_string(tmp_type);
if (Z_STRLEN_P(tmp_type) != 1) {
zend_string *type = zval_try_get_string(tmp_type);
if (!type) {
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
size_t len = ZSTR_LEN(type);
char ptype = *ZSTR_VAL(type);
zend_string_release(type);
if (len != 1) {
zend_value_error("Type must be a single character");
efree(objid_query->vars);
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
pptr = Z_STRVAL_P(tmp_type);
objid_query->vars[objid_query->count].type = *pptr;
objid_query->vars[objid_query->count].type = ptype;
idx_type++;
} else {
php_error_docref(NULL, E_WARNING, "'%s': no type set", Z_STRVAL_P(tmp_oid));
efree(objid_query->vars);
php_error_docref(NULL, E_WARNING, "'%s': no type set", ZSTR_VAL(tmp));
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
}
Expand All @@ -738,12 +774,16 @@ static bool php_snmp_parse_oid(
}
}
if (idx_value < value_ht->nNumUsed) {
convert_to_string(tmp_value);
objid_query->vars[objid_query->count].value = Z_STRVAL_P(tmp_value);
zend_string *tmp = zval_try_get_string(tmp_value);
if (!tmp) {
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
objid_query->vars[objid_query->count].value = ZSTR_VAL(tmp);
idx_value++;
} else {
php_error_docref(NULL, E_WARNING, "'%s': no value set", Z_STRVAL_P(tmp_oid));
efree(objid_query->vars);
php_error_docref(NULL, E_WARNING, "'%s': no value set", ZSTR_VAL(tmp));
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
}
Expand All @@ -756,14 +796,14 @@ static bool php_snmp_parse_oid(
if (st & SNMP_CMD_WALK) {
if (objid_query->count > 1) {
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Multi OID walks are not supported!");
efree(objid_query->vars);
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
objid_query->vars[0].name_length = MAX_NAME_LEN;
if (strlen(objid_query->vars[0].oid)) { /* on a walk, an empty string means top of tree - no error */
if (!snmp_parse_oid(objid_query->vars[0].oid, objid_query->vars[0].name, &(objid_query->vars[0].name_length))) {
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[0].oid);
efree(objid_query->vars);
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
} else {
Expand All @@ -775,7 +815,7 @@ static bool php_snmp_parse_oid(
objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN;
if (!snmp_parse_oid(objid_query->vars[objid_query->offset].oid, objid_query->vars[objid_query->offset].name, &(objid_query->vars[objid_query->offset].name_length))) {
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[objid_query->offset].oid);
efree(objid_query->vars);
php_free_objid_query(objid_query, oid_ht, value_ht, st);
return false;
}
}
Expand Down Expand Up @@ -1252,12 +1292,12 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)

if (session_less_mode) {
if (!netsnmp_session_init(&session, version, a1, a2, timeout, retries)) {
efree(objid_query.vars);
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
netsnmp_session_free(&session);
RETURN_FALSE;
}
if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
efree(objid_query.vars);
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
netsnmp_session_free(&session);
/* Warning message sent already, just bail out */
RETURN_FALSE;
Expand All @@ -1268,7 +1308,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
session = snmp_object->session;
if (!session) {
zend_throw_error(NULL, "Invalid or uninitialized SNMP object");
efree(objid_query.vars);
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
RETURN_THROWS();
}

Expand All @@ -1294,7 +1334,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)

php_snmp_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU, st, session, &objid_query);

efree(objid_query.vars);
php_free_objid_query(&objid_query, oid_ht, value_ht, st);

if (session_less_mode) {
netsnmp_session_free(&session);
Expand Down
69 changes: 69 additions & 0 deletions ext/snmp/tests/gh16959.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
--TEST--
snmpget() modifies object_id array source
--EXTENSIONS--
snmp
--SKIPIF--
<?php
require_once(__DIR__.'/skipif.inc');
?>
--FILE--
<?php
require_once(__DIR__.'/snmp_include.inc');

$bad_object_ids = array (
077 => 077, -066 => -066, -0345 => -0345, 0 => 0
);
var_dump($bad_object_ids);
var_dump(snmpget($hostname, "", $bad_object_ids) === false);
// The array should remain unmodified
var_dump($bad_object_ids);
try {
snmpget($hostname, "", [0 => new stdClass()]);
} catch (Throwable $e) {
echo $e->getMessage() . PHP_EOL;
}

try {
snmp2_set($hostname, $communityWrite, $bad_object_ids, array(new stdClass()), array(null));
} catch (Throwable $e) {
echo $e->getMessage() . PHP_EOL;
}
try {
snmp2_set($hostname, $communityWrite, $bad_object_ids, array("toolongtype"), array(null));
Copy link
Member

Choose a reason for hiding this comment

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

Please also add a test with a non-interned string.
A trick to make one is using str_repeat("my string", random_int(1, 1)).

} catch (Throwable $e) {
echo $e->getMessage() . PHP_EOL;
}
try {
snmp2_set($hostname, $communityWrite, $bad_object_ids, array(str_repeat("onetoomuch", random_int(1, 1))), array(null));
} catch (Throwable $e) {
echo $e->getMessage();
}
?>
--EXPECTF--
array(4) {
[63]=>
int(63)
[-54]=>
int(-54)
[-229]=>
int(-229)
[0]=>
int(0)
}

Warning: snmpget(): Invalid object identifier: -54 in %s on line %d
bool(true)
array(4) {
[63]=>
int(63)
[-54]=>
int(-54)
[-229]=>
int(-229)
[0]=>
int(0)
}
Object of class stdClass could not be converted to string
Object of class stdClass could not be converted to string
Type must be a single character
Type must be a single character
Loading