Skip to content

Commit 215f9a9

Browse files
devnexendevnexen
committed
try out @nielsdos suggestion, delaying free object_id at the end of the call.
1 parent fc4ef43 commit 215f9a9

File tree

2 files changed

+82
-20
lines changed

2 files changed

+82
-20
lines changed

‎ext/snmp/snmp.c‎

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,40 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
626626
}
627627
/* }}} */
628628

629+
static void php_snmp_zend_string_release_from_char_pointer(char *ptr) {
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+
}
636+
}
637+
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_ht && !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) {
652+
snmpobjarg *arg = &objid_query->vars[i];
653+
if (!arg->oid) {
654+
break;
655+
}
656+
PHP_FREE_OBJID_VAL(arg);
657+
i ++;
658+
}
659+
}
660+
efree(objid_query->vars);
661+
}
662+
629663
/* {{{ php_snmp_parse_oid
630664
*
631665
* OID parser (and type, value for SNMP_SET command)
@@ -674,15 +708,15 @@ static bool php_snmp_parse_oid(
674708
return false;
675709
}
676710
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));
677712
objid_query->array_output = (st & SNMP_CMD_SET) == 0;
678713
ZEND_HASH_FOREACH_VAL(oid_ht, tmp_oid) {
679714
zend_string *tmp = zval_try_get_string(tmp_oid);
680715
if (!tmp) {
681-
efree(objid_query->vars);
716+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
682717
return false;
683718
}
684719
objid_query->vars[objid_query->count].oid = ZSTR_VAL(tmp);
685-
zend_string_release(tmp);
686720
if (st & SNMP_CMD_SET) {
687721
if (type_str) {
688722
pptr = ZSTR_VAL(type_str);
@@ -706,18 +740,23 @@ static bool php_snmp_parse_oid(
706740
}
707741
}
708742
if (idx_type < type_ht->nNumUsed) {
709-
convert_to_string(tmp_type);
710-
if (Z_STRLEN_P(tmp_type) != 1) {
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);
746+
return false;
747+
}
748+
if (ZSTR_LEN(type) != 1) {
711749
zend_value_error("Type must be a single character");
712-
efree(objid_query->vars);
750+
zend_string_release(type);
751+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
713752
return false;
714753
}
715-
pptr = Z_STRVAL_P(tmp_type);
754+
pptr = ZSTR_VAL(type);
716755
objid_query->vars[objid_query->count].type = *pptr;
717756
idx_type++;
718757
} else {
719-
php_error_docref(NULL, E_WARNING, "'%s': no type set", Z_STRVAL_P(tmp_oid));
720-
efree(objid_query->vars);
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);
721760
return false;
722761
}
723762
}
@@ -743,12 +782,16 @@ static bool php_snmp_parse_oid(
743782
}
744783
}
745784
if (idx_value < value_ht->nNumUsed) {
746-
convert_to_string(tmp_value);
747-
objid_query->vars[objid_query->count].value = Z_STRVAL_P(tmp_value);
785+
zend_string *tmp = zval_try_get_string(tmp_value);
786+
if (!tmp) {
787+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
788+
return false;
789+
}
790+
objid_query->vars[objid_query->count].value = ZSTR_VAL(tmp);
748791
idx_value++;
749792
} else {
750-
php_error_docref(NULL, E_WARNING, "'%s': no value set", Z_STRVAL_P(tmp_oid));
751-
efree(objid_query->vars);
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);
752795
return false;
753796
}
754797
}
@@ -761,14 +804,14 @@ static bool php_snmp_parse_oid(
761804
if (st & SNMP_CMD_WALK) {
762805
if (objid_query->count > 1) {
763806
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Multi OID walks are not supported!");
764-
efree(objid_query->vars);
807+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
765808
return false;
766809
}
767810
objid_query->vars[0].name_length = MAX_NAME_LEN;
768811
if (strlen(objid_query->vars[0].oid)) { /* on a walk, an empty string means top of tree - no error */
769812
if (!snmp_parse_oid(objid_query->vars[0].oid, objid_query->vars[0].name, &(objid_query->vars[0].name_length))) {
770813
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[0].oid);
771-
efree(objid_query->vars);
814+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
772815
return false;
773816
}
774817
} else {
@@ -780,7 +823,7 @@ static bool php_snmp_parse_oid(
780823
objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN;
781824
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))) {
782825
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[objid_query->offset].oid);
783-
efree(objid_query->vars);
826+
php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
784827
return false;
785828
}
786829
}
@@ -1257,12 +1300,12 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12571300

12581301
if (session_less_mode) {
12591302
if (!netsnmp_session_init(&session, version, a1, a2, timeout, retries)) {
1260-
efree(objid_query.vars);
1303+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
12611304
netsnmp_session_free(&session);
12621305
RETURN_FALSE;
12631306
}
12641307
if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
1265-
efree(objid_query.vars);
1308+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
12661309
netsnmp_session_free(&session);
12671310
/* Warning message sent already, just bail out */
12681311
RETURN_FALSE;
@@ -1273,7 +1316,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12731316
session = snmp_object->session;
12741317
if (!session) {
12751318
zend_throw_error(NULL, "Invalid or uninitialized SNMP object");
1276-
efree(objid_query.vars);
1319+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
12771320
RETURN_THROWS();
12781321
}
12791322

@@ -1299,7 +1342,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12991342

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

1302-
efree(objid_query.vars);
1345+
php_free_objid_query(&objid_query, oid_ht, value_str, value_ht, st);
13031346

13041347
if (session_less_mode) {
13051348
netsnmp_session_free(&session);

‎ext/snmp/tests/gh16959.phpt‎

Lines changed: 20 additions & 1 deletion
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
}
@@ -35,7 +51,7 @@ array(4) {
3551
int(0)
3652
}
3753

38-
Warning: snmpget(): Invalid object identifier: -229 in %s on line %d
54+
Warning: snmpget(): Invalid object identifier: -54 in %s on line %d
3955
bool(true)
4056
array(4) {
4157
[63]=>
@@ -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)