]> The Tcpdump Group git mirrors - tcpdump/commitdiff
ICMP: Fix an undefined behavior for the Interface Name Sub-Object
authorFrancois-Xavier Le Bail <[email protected]>
Mon, 18 Dec 2023 17:12:05 +0000 (18:12 +0100)
committerFrancois-Xavier Le Bail <[email protected]>
Mon, 18 Dec 2023 17:34:09 +0000 (18:34 +0100)
Add a test for the Interface Name Sub-Object length == 0.
If inft_name_length_field == 0, nd_printjnp() was called with
inft_name_length_field - 1 == -1.

Add a test file.

The error was:
print-icmp.c:893:37: runtime error: implicit conversion from type 'int'
of value -1 (32-bit, signed) to type 'u_int' (aka 'unsigned int')
changed the value to 4294967295 (32-bit, unsigned)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior print-icmp.c:893:37

print-icmp.c
tests/TESTLIST
tests/icmp_inft_name_length_zero.out [new file with mode: 0644]
tests/icmp_inft_name_length_zero.pcap [new file with mode: 0644]

index 074d937716c3927811eb53987f027ce84bd2e503..6f962500e36eea396ce9e40ac3227f9285edb0aa 100644 (file)
@@ -876,6 +876,11 @@ icmp_print(netdissect_options *ndo, const u_char *bp, u_int plen, const u_char *
                         ifname_subobj = (const struct icmp_interface_identification_ifname_subobject_t *) offset;
                         inft_name_length_field = GET_U_1(ifname_subobj->length);
                         ND_PRINT("\n\t\t Interface Name");
+                        if (inft_name_length_field == 0) {
+                            ND_PRINT(" [length %u]", inft_name_length_field);
+                            nd_print_invalid(ndo);
+                            break;
+                        }
                         if (inft_name_length_field % 4 != 0) {
                             ND_PRINT(" [length %u != N x 4]", inft_name_length_field);
                             nd_print_invalid(ndo);
index a9e4be1c3a3fe55784d506769dbe053acb11d1bb..259ce1d8f5cd58b292f86592f2b31dbdfbc1279b 100644 (file)
@@ -215,6 +215,7 @@ dvmrp               mrinfo_query.pcap       dvmrp.out
 
 # ICMPv4 -- pcap from https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6632
 rfc5837    icmp-rfc5837.pcap icmp-rfc5837.out -v
+icmp_inft_name_length_zero icmp_inft_name_length_zero.pcap icmp_inft_name_length_zero.out -v
 
 # ICMPv6
 icmpv6          icmpv6.pcap             icmpv6.out      -vv
diff --git a/tests/icmp_inft_name_length_zero.out b/tests/icmp_inft_name_length_zero.out
new file mode 100644 (file)
index 0000000..3e16a4d
--- /dev/null
@@ -0,0 +1,8 @@
+    1  10:13:29.4294643617 IP [total length 33008 > length 240] (invalid) (tos 0x0, ttl 254, id 59168, offset 0, flags [DF], proto ICMP (1), length 33008, bad cksum 7ade (->464)!)
+    0.128.255.255 > 12.4.4.4: ICMP time exceeded in-transit, length 32988
+       IP (tos 0x0, ttl 1, id 42321, offset 0, flags [none], proto UDP (17), length 40, bad cksum f76a (->db81)!)
+    8.15.4.4.42315 > 12.223.32.1.33440: UDP, length 12
+       ICMP Multi-Part extension v2
+         Interface Identification Object (2), Class-Type: 2, length 8016
+                This object describes the IP interface upon which a datagram arrived
+                Interface Name [length 0] (invalid) [|icmp]
diff --git a/tests/icmp_inft_name_length_zero.pcap b/tests/icmp_inft_name_length_zero.pcap
new file mode 100644 (file)
index 0000000..dadb6d1
Binary files /dev/null and b/tests/icmp_inft_name_length_zero.pcap differ