From df5cba8d99ed9b99ecf1da9e4e0a115509209a91 Mon Sep 17 00:00:00 2001 From: Bill Fenner Date: Tue, 11 Oct 2022 13:13:58 -0700 Subject: [PATCH] SNMP: Fix two undefined behaviors 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 #1012] --- CHANGES | 1 + print-snmp.c | 8 +++++--- tests/TESTLIST | 4 ++++ tests/ip-snmp-leftshift-unsigned.out | 1 + tests/ip-snmp-leftshift-unsigned.pcap | Bin 0 -> 137 bytes tests/ip6-snmp-oid-unsigned.out | 1 + tests/ip6-snmp-oid-unsigned.pcap | Bin 0 -> 209 bytes 7 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 tests/ip-snmp-leftshift-unsigned.out create mode 100644 tests/ip-snmp-leftshift-unsigned.pcap create mode 100644 tests/ip6-snmp-oid-unsigned.out create mode 100644 tests/ip6-snmp-oid-unsigned.pcap diff --git a/CHANGES b/CHANGES index 91e087e20..54dd4896f 100644 --- a/CHANGES +++ b/CHANGES @@ -27,6 +27,7 @@ DayOfTheWeek, Month DD, YYYY / The Tcpdump Group BGP: Add support for BGP Role capability and OTC attribute Arista: Use the test .pcap file from pull request #955 (HwInfo). NFLOG: Use correct AF code points on all OSes. + SNMP: Fix two undefined behaviors. User interface: Add optional unit suffix on -C file size. Add --print-sampling to print every Nth packet instead of all. diff --git a/print-snmp.c b/print-snmp.c index 1fc08f380..42c9785ae 100644 --- a/print-snmp.c +++ b/print-snmp.c @@ -66,6 +66,7 @@ #include #include +#include #ifdef USE_LIBSMI #include @@ -531,7 +532,7 @@ asn1_parse(netdissect_options *ndo, break; case INTEGER: { - int32_t data; + uint32_t data; elem->type = BE_INT; data = 0; @@ -540,7 +541,7 @@ asn1_parse(netdissect_options *ndo, goto invalid; } 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; @@ -726,7 +727,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; diff --git a/tests/TESTLIST b/tests/TESTLIST index 46babe8db..6dd75921d 100644 --- a/tests/TESTLIST +++ b/tests/TESTLIST @@ -918,3 +918,7 @@ NHRP_registration NHRP_registration.pcap NHRP_registration.out -v NHRP-responder-address NHRP-responder-address.pcap NHRP-responder-address.out -v nhrp-trace nhrp-trace.pcap nhrp-trace.out -v nhrp nhrp.pcap nhrp.out -v + +# 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 diff --git a/tests/ip-snmp-leftshift-unsigned.out b/tests/ip-snmp-leftshift-unsigned.out new file mode 100644 index 000000000..eaf0779b2 --- /dev/null +++ b/tests/ip-snmp-leftshift-unsigned.out @@ -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 diff --git a/tests/ip-snmp-leftshift-unsigned.pcap b/tests/ip-snmp-leftshift-unsigned.pcap new file mode 100644 index 0000000000000000000000000000000000000000..de897628de8371c593072fa12a7af1b3757bf0a1 GIT binary patch literal 137 zcmca|c+)~A1{MYw`2U}Qff2|#Xl 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 diff --git a/tests/ip6-snmp-oid-unsigned.pcap b/tests/ip6-snmp-oid-unsigned.pcap new file mode 100644 index 0000000000000000000000000000000000000000..aeed213503c0826a5d5d13024a5090c033250826 GIT binary patch literal 209 zcmca|c+)~A1{MYw`2U}Qff2}gAa9=ZC5nw