]> The Tcpdump Group git mirrors - tcpdump/commitdiff
SNMP: Fix two undefined behaviors
authorBill Fenner <[email protected]>
Tue, 11 Oct 2022 20:13:58 +0000 (13:13 -0700)
committerFrancois-Xavier Le Bail <[email protected]>
Thu, 12 Oct 2023 12:35:19 +0000 (14:35 +0200)
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]

(cherry picked from commit df5cba8d99ed9b99ecf1da9e4e0a115509209a91)

print-snmp.c
tests/TESTLIST
tests/ip-snmp-leftshift-unsigned.out [new file with mode: 0644]
tests/ip-snmp-leftshift-unsigned.pcap [new file with mode: 0644]
tests/ip6-snmp-oid-unsigned.out [new file with mode: 0644]
tests/ip6-snmp-oid-unsigned.pcap [new file with mode: 0644]

index 6aae34caa7de1b90ab54557e30feb66c7cd90bcf..d05a5d383dd413b45d5dd67654ad1734147545e7 100644 (file)
@@ -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,
                                        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;
@@ -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;
index 6d5768f05428d4b427089978bd4ab74413c838d3..743512cae54d905916c768c37a0ba4eeeba890b0 100644 (file)
@@ -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
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
@@ -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
@@ -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