Skip to content

Commit 9b32d54

Browse files
ext/snmp: hardening: replace unbounded strcat with offset-tracked snprintf
The suffix-as-keys code path in php_snmp builds a dotted OID suffix string by appending with strcat into a fixed 2048-byte buf2 array. The overflow is not reachable in practice: net-snmp's BER decoder enforces MAX_OID_LEN=128, and the worst-case suffix (128 subidentifiers at 11 bytes each) is 1408 bytes, within the buffer. Replace the strcat loop with direct snprintf into buf2 at a tracked size_t offset, breaking out of the loop if the buffer would be exhausted. Defense-in-depth against future net-snmp parser regressions. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
1 parent 11a9574 commit 9b32d54

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

ext/snmp/snmp.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -525,14 +525,21 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
525525
} else if (st & SNMP_USE_SUFFIX_AS_KEYS && st & SNMP_CMD_WALK) {
526526
snprint_objid(buf2, sizeof(buf2), vars->name, vars->name_length);
527527
if (rootlen <= vars->name_length && snmp_oid_compare(root, rootlen, vars->name, rootlen) == 0) {
528-
buf2[0] = '\0';
528+
size_t pos = 0;
529529
count = rootlen;
530-
while(count < vars->name_length){
531-
snprintf(buf, sizeof(buf), "%lu.", vars->name[count]);
532-
strcat(buf2, buf);
530+
while (count < vars->name_length) {
531+
int written = snprintf(buf2 + pos, sizeof(buf2) - pos, "%lu.", vars->name[count]);
532+
if (written < 0 || (size_t)written >= sizeof(buf2) - pos) {
533+
break;
534+
}
535+
pos += (size_t)written;
533536
count++;
534537
}
535-
buf2[strlen(buf2) - 1] = '\0'; /* remove trailing '.' */
538+
if (pos > 0) {
539+
buf2[pos - 1] = '\0'; /* remove trailing '.' */
540+
} else {
541+
buf2[0] = '\0';
542+
}
536543
}
537544
add_assoc_zval(return_value, buf2, &snmpval);
538545
} else {

0 commit comments

Comments
 (0)