Skip to content

Commit 30c826c

Browse files
devnexendevnexen
committed
triggers another ZendMM leak
1 parent e2691d8 commit 30c826c

File tree

2 files changed

+65
-31
lines changed

2 files changed

+65
-31
lines changed

‎ext/snmp/snmp.c‎

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -627,20 +627,34 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
627627
/* }}} */
628628

629629
static void php_snmp_zend_string_release_from_char_pointer(char *ptr) {
630-
zend_string *pptr = (zend_string *)(ptr - XtOffsetOf(zend_string, val));
631-
zend_string_release(pptr);
630+
if (ptr) {
631+
zend_string *pptr = (zend_string *)(ptr - XtOffsetOf(zend_string, val));
632+
if (GC_REFCOUNT(pptr)) {
633+
zend_string_release(pptr);
634+
}
635+
}
632636
}
633637

634-
static void php_free_objid_query(struct objid_query *objid_query, zend_string* oid_str, zend_string *value_str, HashTable *value_ht, int st) {
635-
if (!oid_str) {
636-
for (int i = 0; i < objid_query->count; i ++) {
638+
static void php_free_objid_query(struct objid_query *objid_query, HashTable* oid_ht, zend_string *value_str, HashTable *value_ht, int st) {
639+
#define PHP_FREE_OBJID_VAL(arg) \
640+
do { \
641+
if (value_str) { \
642+
php_snmp_zend_string_release_from_char_pointer(arg->value); \
643+
} \
644+
php_snmp_zend_string_release_from_char_pointer(&arg->type); \
645+
php_snmp_zend_string_release_from_char_pointer(arg->oid); \
646+
} while (0)
647+
648+
if (oid_ht) {
649+
uint32_t i = 0, count = zend_hash_num_elements(oid_ht);
650+
651+
while (i < count) {
637652
snmpobjarg *arg = &objid_query->vars[i];
638-
if (st & SNMP_CMD_SET) {
639-
if (!value_str && value_ht) {
640-
php_snmp_zend_string_release_from_char_pointer(arg->value);
641-
}
653+
if (!arg->oid) {
654+
break;
642655
}
643-
php_snmp_zend_string_release_from_char_pointer(arg->oid);
656+
PHP_FREE_OBJID_VAL(arg);
657+
i ++;
644658
}
645659
}
646660
efree(objid_query->vars);
@@ -694,11 +708,12 @@ static bool php_snmp_parse_oid(
694708
return false;
695709
}
696710
objid_query->vars = (snmpobjarg *)safe_emalloc(sizeof(snmpobjarg), zend_hash_num_elements(oid_ht), 0);
711+
memset(objid_query->vars, 0, sizeof(snmpobjarg) * zend_hash_num_elements(oid_ht));
697712
objid_query->array_output = (st & SNMP_CMD_SET) == 0;
698713
ZEND_HASH_FOREACH_VAL(oid_ht, tmp_oid) {
699714
zend_string *tmp = zval_try_get_string(tmp_oid);
700715
if (!tmp) {
701-
efree(objid_query->vars);
716+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
702717
return false;
703718
}
704719
objid_query->vars[objid_query->count].oid = ZSTR_VAL(tmp);
@@ -725,23 +740,23 @@ static bool php_snmp_parse_oid(
725740
}
726741
}
727742
if (idx_type < type_ht->nNumUsed) {
728-
zval new;
729-
ZVAL_COPY_VALUE(&new, tmp_type);
730-
if (!try_convert_to_string(&new)) {
731-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
743+
zend_string *type = zval_try_get_string(tmp_type);
744+
if (!type) {
745+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
732746
return false;
733747
}
734-
if (Z_STRLEN(new) != 1) {
748+
if (ZSTR_LEN(type) != 1) {
735749
zend_value_error("Type must be a single character");
736-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
750+
zend_string_release(type);
751+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
737752
return false;
738753
}
739-
pptr = Z_STRVAL(new);
754+
pptr = ZSTR_VAL(type);
740755
objid_query->vars[objid_query->count].type = *pptr;
741756
idx_type++;
742757
} else {
743-
php_error_docref(NULL, E_WARNING, "'%s': no type set", Z_STRVAL_P(tmp_oid));
744-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
758+
php_error_docref(NULL, E_WARNING, "'%s': no type set", ZSTR_VAL(tmp));
759+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
745760
return false;
746761
}
747762
}
@@ -769,14 +784,14 @@ static bool php_snmp_parse_oid(
769784
if (idx_value < value_ht->nNumUsed) {
770785
zend_string *tmp = zval_try_get_string(tmp_value);
771786
if (!tmp) {
772-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
787+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
773788
return false;
774789
}
775790
objid_query->vars[objid_query->count].value = ZSTR_VAL(tmp);
776791
idx_value++;
777792
} else {
778-
php_error_docref(NULL, E_WARNING, "'%s': no value set", Z_STRVAL_P(tmp_oid));
779-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
793+
php_error_docref(NULL, E_WARNING, "'%s': no value set", ZSTR_VAL(tmp));
794+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
780795
return false;
781796
}
782797
}
@@ -789,14 +804,14 @@ static bool php_snmp_parse_oid(
789804
if (st & SNMP_CMD_WALK) {
790805
if (objid_query->count > 1) {
791806
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Multi OID walks are not supported!");
792-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
807+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
793808
return false;
794809
}
795810
objid_query->vars[0].name_length = MAX_NAME_LEN;
796811
if (strlen(objid_query->vars[0].oid)) { /* on a walk, an empty string means top of tree - no error */
797812
if (!snmp_parse_oid(objid_query->vars[0].oid, objid_query->vars[0].name, &(objid_query->vars[0].name_length))) {
798813
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[0].oid);
799-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
814+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
800815
return false;
801816
}
802817
} else {
@@ -808,7 +823,7 @@ static bool php_snmp_parse_oid(
808823
objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN;
809824
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))) {
810825
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[objid_query->offset].oid);
811-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
826+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
812827
return false;
813828
}
814829
}
@@ -1285,12 +1300,12 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12851300

12861301
if (session_less_mode) {
12871302
if (!netsnmp_session_init(&session, version, a1, a2, timeout, retries)) {
1288-
php_free_objid_query(&objid_query, oid_str, value_str, value_ht, st);
1303+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
12891304
netsnmp_session_free(&session);
12901305
RETURN_FALSE;
12911306
}
12921307
if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
1293-
php_free_objid_query(&objid_query, oid_str, value_str, value_ht, st);
1308+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
12941309
netsnmp_session_free(&session);
12951310
/* Warning message sent already, just bail out */
12961311
RETURN_FALSE;
@@ -1301,7 +1316,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13011316
session = snmp_object->session;
13021317
if (!session) {
13031318
zend_throw_error(NULL, "Invalid or uninitialized SNMP object");
1304-
php_free_objid_query(&objid_query, oid_str, value_str, value_ht, st);
1319+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
13051320
RETURN_THROWS();
13061321
}
13071322

@@ -1327,15 +1342,15 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13271342

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

1345+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
1346+
13301347
if (session_less_mode) {
13311348
netsnmp_session_free(&session);
13321349
} else {
13331350
netsnmp_ds_set_boolean(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_PRINT_NUMERIC_ENUM, glob_snmp_object.enum_print);
13341351
netsnmp_ds_set_boolean(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_QUICK_PRINT, glob_snmp_object.quick_print);
13351352
netsnmp_ds_set_int(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_OID_OUTPUT_FORMAT, glob_snmp_object.oid_output_format);
13361353
}
1337-
1338-
php_free_objid_query(&objid_query, oid_str, value_str, value_ht, st);
13391354
}
13401355
/* }}} */
13411356

‎ext/snmp/tests/gh16959.phpt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,22 @@ var_dump(snmpget($hostname, "", $bad_object_ids) === false);
1919
var_dump($bad_object_ids);
2020
try {
2121
snmpget($hostname, "", [0 => new stdClass()]);
22+
} catch (Throwable $e) {
23+
echo $e->getMessage() . PHP_EOL;
24+
}
25+
26+
try {
27+
snmp2_set($hostname, $communityWrite, $bad_object_ids, array(new stdClass()), array(null));
28+
} catch (Throwable $e) {
29+
echo $e->getMessage() . PHP_EOL;
30+
}
31+
try {
32+
snmp2_set($hostname, $communityWrite, $bad_object_ids, array("toolongtype"), array(null));
33+
} catch (Throwable $e) {
34+
echo $e->getMessage() . PHP_EOL;
35+
}
36+
try {
37+
snmp2_set($hostname, $communityWrite, $bad_object_ids, array(str_repeat("onetoomuch", random_int(1, 1))), array(null));
2238
} catch (Throwable $e) {
2339
echo $e->getMessage();
2440
}
@@ -48,3 +64,6 @@ array(4) {
4864
int(0)
4965
}
5066
Object of class stdClass could not be converted to string
67+
Object of class stdClass could not be converted to string
68+
Type must be a single character
69+
Type must be a single character

0 commit comments

Comments
 (0)