Skip to content

Commit

Permalink
SNMP: Fix two undefined behaviors
Browse files Browse the repository at this point in the history
When converting an integer from ASN.1, use an unsigned value
for the partial result and assign it to the integer part of
the union at the end, to avoid shifting a negative number left.

print-snmp.c:545:19: runtime error: left shift of negative value -1
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior print-snmp.c:545:19

OID elements are unsigned; a large-enough oid value could result
in the undefined behavior of shifting a signed integer left through
the sign bit, so simply store them as unsigned.

print-snmp.c:751:11: runtime error: left shift of 268435455 by 7 places
  cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior print-snmp.c:751:11

[Part of the PR the-tcpdump-group#1012]

(cherry picked from commit df5cba8)
  • Loading branch information
fenner authored and fxlb committed Oct 12, 2023
1 parent 1ba8a39 commit 36d0544
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 3 deletions.
8 changes: 5 additions & 3 deletions print-snmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@

#include <stdio.h>
#include <string.h>
#include <limits.h>

#ifdef USE_LIBSMI
#include <smi.h>
Expand Down Expand Up @@ -531,7 +532,7 @@ asn1_parse(netdissect_options *ndo,
break;

case INTEGER: {
int32_t data;
uint32_t data;
elem->type = BE_INT;
data = 0;

Expand All @@ -540,7 +541,7 @@ asn1_parse(netdissect_options *ndo,
return -1;
}
if (GET_U_1(p) & ASN_BIT8) /* negative */
data = -1;
data = UINT_MAX;
for (i = elem->asnlen; i != 0; p++, i--)
data = (data << ASN_SHIFT8) | GET_U_1(p);
elem->data.integer = data;
Expand Down Expand Up @@ -743,7 +744,8 @@ asn1_print(netdissect_options *ndo,
break;

case BE_OID: {
int o = 0, first = -1;
int first = -1;
uint32_t o = 0;

p = (const u_char *)elem->data.raw;
i = asnlen;
Expand Down
4 changes: 4 additions & 0 deletions tests/TESTLIST
Original file line number Diff line number Diff line change
Expand Up @@ -852,3 +852,7 @@ lsp-ping-timestamp lsp-ping-timestamp.pcap lsp-ping-timestamp.out -vv

# lwres with "extra" bytes
lwres_with_extra lwres_with_extra.pcap lwres_with_extra.out

# Undefined behavior tests
ip-snmp-leftshift-unsigned ip-snmp-leftshift-unsigned.pcap ip-snmp-leftshift-unsigned.out
ip6-snmp-oid-unsigned ip6-snmp-oid-unsigned.pcap ip6-snmp-oid-unsigned.out
1 change: 1 addition & 0 deletions tests/ip-snmp-leftshift-unsigned.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1 14:35:45.695106 IP truncated-ip - 34734 bytes missing! 10.0.0.0.162 > 154.1.214.234.65535: [!init SEQ]-1
Binary file added tests/ip-snmp-leftshift-unsigned.pcap
Binary file not shown.
1 change: 1 addition & 0 deletions tests/ip6-snmp-oid-unsigned.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1 12:36:48.416500 IP6 fe80::20c:29ff:fe9b:a15d.161 > fe80::20c:0:0:0.546: [!init SEQ].1.11.1.99.0.0.0.0.0.0.4.4.71.8327.1936855.0.1.0.14.0.1.0.1.24.14347.63.0.12.41.57.14824.0.2.0.14.0.1.0.1.24.1821339.0.12.41.446675.0.56.0.61.0.11.0.16.42.1.0.0.4294967168.0.0.0.0.0.0.0.0.0.0.1.0.2.3.110.116.112.7.101.120.97
Binary file added tests/ip6-snmp-oid-unsigned.pcap
Binary file not shown.

0 comments on commit 36d0544

Please sign in to comment.