Skip to content

Commit 6a687cf

Browse files
fennerfxlb
authored andcommitted
Avoid undefined behavior when handling large ASN.1 integers
Instead of shifting bits off the top of the 32-bit value, we mask off the top 8 bits before shifting them away, and restore the sign bit at the end. This still results in a result that is not what was intended, as this code can not handle values greater than 2^31-1 or smaller than -2^31, but this new mechanism results in a "more correct" garbage out, with no undefined behavior.
1 parent ae11515 commit 6a687cf

File tree

1 file changed

+8
-3
lines changed

1 file changed

+8
-3
lines changed

print-snmp.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -531,18 +531,23 @@ asn1_parse(netdissect_options *ndo,
531531

532532
case INTEGER: {
533533
uint32_t data;
534+
uint32_t signbit = 0;
534535
elem->type = BE_INT;
535536
data = 0;
536537

537538
if (elem->asnlen == 0) {
538539
ND_PRINT("[asnlen=0]");
539540
goto invalid;
540541
}
541-
if (GET_U_1(p) & ASN_BIT8) /* negative */
542+
if (GET_U_1(p) & ASN_BIT8) { /* negative */
542543
data = UINT_MAX;
544+
signbit = 0x80000000;
545+
}
546+
/* We can only store the low 32 bits of the given value. If we've
547+
* shifted the sign bit away, we will restore it below. */
543548
for (i = elem->asnlen; i != 0; p++, i--)
544-
data = (data << ASN_SHIFT8) | GET_U_1(p);
545-
elem->data.integer = data;
549+
data = ( ( data & 0x00ffffff ) << ASN_SHIFT8) | GET_U_1(p);
550+
elem->data.integer = (int32_t)( data | signbit );
546551
break;
547552
}
548553

0 commit comments

Comments
 (0)