From: Francois-Xavier Le Bail Date: Mon, 18 Dec 2023 17:12:05 +0000 (+0100) Subject: ICMP: Fix an undefined behavior for the Interface Name Sub-Object X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/5fefba15ae17eadae4c073385aa0971775039cb1 ICMP: Fix an undefined behavior for the Interface Name Sub-Object 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 --- diff --git a/print-icmp.c b/print-icmp.c index 074d9377..6f962500 100644 --- a/print-icmp.c +++ b/print-icmp.c @@ -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); diff --git a/tests/TESTLIST b/tests/TESTLIST index a9e4be1c..259ce1d8 100644 --- a/tests/TESTLIST +++ b/tests/TESTLIST @@ -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 index 00000000..3e16a4df --- /dev/null +++ b/tests/icmp_inft_name_length_zero.out @@ -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 index 00000000..dadb6d19 Binary files /dev/null and b/tests/icmp_inft_name_length_zero.pcap differ