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]

diff --git a/CHANGES b/CHANGES
index 91e087e20ad9938957f2ef27af217fc606bf6359..54dd4896fea31b4c4d9db3f65423dbefbab81607 100644 (file)
--- 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 1fc08f380515ef1360337097eeeb05ad84c38503..42c9785ae25de18ce0832a8471b20f0a45daf72d 100644 (file)
--- a/print-snmp.c
+++ b/print-snmp.c
@@ -66,6 +66,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <limits.h>
 
 #ifdef USE_LIBSMI
 #include <smi.h>
@@ -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 46babe8db8de45cf986057e80f9654395d3d4e44..6dd75921da97131046dbf61e823718b890441103 100644 (file)
--- 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 (file)
index 0000000..eaf0779
--- /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 (file)
index 0000000..de89762
Binary files /dev/null and b/tests/ip-snmp-leftshift-unsigned.pcap differ

diff --git a/tests/ip6-snmp-oid-unsigned.out b/tests/ip6-snmp-oid-unsigned.out
new file mode 100644 (file)
index 0000000..98aa601
--- /dev/null
+++ b/tests/ip6-snmp-oid-unsigned.out
@@ -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

diff --git a/tests/ip6-snmp-oid-unsigned.pcap b/tests/ip6-snmp-oid-unsigned.pcap
new file mode 100644 (file)
index 0000000..aeed213
Binary files /dev/null and b/tests/ip6-snmp-oid-unsigned.pcap differ