Skip to content

Commit 389512b

Browse files
devnexendevnexen
committed
triggers another ZendMM leak
1 parent e2691d8 commit 389512b

File tree

2 files changed

+68
-30
lines changed

2 files changed

+68
-30
lines changed

‎ext/snmp/snmp.c‎

Lines changed: 49 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -627,20 +627,38 @@ 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+
} else {
635+
efree(pptr);
636+
}
637+
}
632638
}
633639

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 ++) {
640+
static void php_free_objid_query(struct objid_query *objid_query, HashTable* oid_ht, zend_string *value_str, HashTable *value_ht, int st) {
641+
#define PHP_FREE_OBJID_VAL(arg) \
642+
do { \
643+
if (st & SNMP_CMD_SET) { \
644+
if (value_str) { \
645+
php_snmp_zend_string_release_from_char_pointer(arg->value); \
646+
} \
647+
php_snmp_zend_string_release_from_char_pointer(&arg->type); \
648+
} \
649+
php_snmp_zend_string_release_from_char_pointer(arg->oid); \
650+
} while (0)
651+
652+
if (oid_ht) {
653+
uint32_t i = 0, count = zend_hash_num_elements(oid_ht);
654+
655+
while (i < count) {
637656
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-
}
657+
if (!arg->oid) {
658+
break;
642659
}
643-
php_snmp_zend_string_release_from_char_pointer(arg->oid);
660+
PHP_FREE_OBJID_VAL(arg);
661+
i ++;
644662
}
645663
}
646664
efree(objid_query->vars);
@@ -694,11 +712,12 @@ static bool php_snmp_parse_oid(
694712
return false;
695713
}
696714
objid_query->vars = (snmpobjarg *)safe_emalloc(sizeof(snmpobjarg), zend_hash_num_elements(oid_ht), 0);
715+
memset(objid_query->vars, 0, sizeof(snmpobjarg) * zend_hash_num_elements(oid_ht));
697716
objid_query->array_output = (st & SNMP_CMD_SET) == 0;
698717
ZEND_HASH_FOREACH_VAL(oid_ht, tmp_oid) {
699718
zend_string *tmp = zval_try_get_string(tmp_oid);
700719
if (!tmp) {
701-
efree(objid_query->vars);
720+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
702721
return false;
703722
}
704723
objid_query->vars[objid_query->count].oid = ZSTR_VAL(tmp);
@@ -725,23 +744,23 @@ static bool php_snmp_parse_oid(
725744
}
726745
}
727746
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);
747+
zend_string *type = zval_try_get_string(tmp_type);
748+
if (!type) {
749+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
732750
return false;
733751
}
734-
if (Z_STRLEN(new) != 1) {
752+
if (ZSTR_LEN(type) != 1) {
735753
zend_value_error("Type must be a single character");
736-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
754+
zend_string_release(type);
755+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
737756
return false;
738757
}
739-
pptr = Z_STRVAL(new);
758+
pptr = ZSTR_VAL(type);
740759
objid_query->vars[objid_query->count].type = *pptr;
741760
idx_type++;
742761
} 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);
762+
php_error_docref(NULL, E_WARNING, "'%s': no type set", ZSTR_VAL(tmp));
763+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
745764
return false;
746765
}
747766
}
@@ -769,14 +788,14 @@ static bool php_snmp_parse_oid(
769788
if (idx_value < value_ht->nNumUsed) {
770789
zend_string *tmp = zval_try_get_string(tmp_value);
771790
if (!tmp) {
772-
php_free_objid_query(objid_query, oid_str, value_str, value_ht, st);
791+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
773792
return false;
774793
}
775794
objid_query->vars[objid_query->count].value = ZSTR_VAL(tmp);
776795
idx_value++;
777796
} 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);
797+
php_error_docref(NULL, E_WARNING, "'%s': no value set", ZSTR_VAL(tmp));
798+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
780799
return false;
781800
}
782801
}
@@ -789,14 +808,14 @@ static bool php_snmp_parse_oid(
789808
if (st & SNMP_CMD_WALK) {
790809
if (objid_query->count > 1) {
791810
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);
811+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
793812
return false;
794813
}
795814
objid_query->vars[0].name_length = MAX_NAME_LEN;
796815
if (strlen(objid_query->vars[0].oid)) { /* on a walk, an empty string means top of tree - no error */
797816
if (!snmp_parse_oid(objid_query->vars[0].oid, objid_query->vars[0].name, &(objid_query->vars[0].name_length))) {
798817
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);
818+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
800819
return false;
801820
}
802821
} else {
@@ -808,7 +827,7 @@ static bool php_snmp_parse_oid(
808827
objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN;
809828
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))) {
810829
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);
830+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
812831
return false;
813832
}
814833
}
@@ -1285,12 +1304,12 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12851304

12861305
if (session_less_mode) {
12871306
if (!netsnmp_session_init(&session, version, a1, a2, timeout, retries)) {
1288-
php_free_objid_query(&objid_query, oid_str, value_str, value_ht, st);
1307+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
12891308
netsnmp_session_free(&session);
12901309
RETURN_FALSE;
12911310
}
12921311
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);
1312+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
12941313
netsnmp_session_free(&session);
12951314
/* Warning message sent already, just bail out */
12961315
RETURN_FALSE;
@@ -1301,7 +1320,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13011320
session = snmp_object->session;
13021321
if (!session) {
13031322
zend_throw_error(NULL, "Invalid or uninitialized SNMP object");
1304-
php_free_objid_query(&objid_query, oid_str, value_str, value_ht, st);
1323+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
13051324
RETURN_THROWS();
13061325
}
13071326

@@ -1335,7 +1354,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13351354
netsnmp_ds_set_int(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_OID_OUTPUT_FORMAT, glob_snmp_object.oid_output_format);
13361355
}
13371356

1338-
php_free_objid_query(&objid_query, oid_str, value_str, value_ht, st);
1357+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
13391358
}
13401359
/* }}} */
13411360

‎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)