From: Francois-Xavier Le Bail Date: Mon, 7 Feb 2022 14:51:36 +0000 (+0100) Subject: ICMP: Fix printing the Interface Name Sub-Object X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/f30baa4292317ec709c02b787945d86fce17fa7e ICMP: Fix printing the Interface Name Sub-Object RFC 5837 - 4.3. Interface Name Sub-Object "The Interface Name Sub-Object MUST have a length that is a multiple of 4 octets and MUST NOT exceed 64 octets. The Length field represents the length of the Interface Name Sub- Object, including the length and the interface name in octets." The length of the interface name to print is: (Length field) - 1. The offset is only: Length field Add sanity checks: Multiple of 4 octets, <= 64 octets. Fix the icmp-rfc5837.pcap test. The interface name length in octets is 63, thus the length field must be 64. Update also the ICMP Multi-Part Extensions checksum. Keep an invalid length field in icmp-cksum-oobr-2.pcap. Update the output of two tests accordingly. --- diff --git a/print-icmp.c b/print-icmp.c index 4e66a3cf..74f327fc 100644 --- a/print-icmp.c +++ b/print-icmp.c @@ -853,11 +853,23 @@ 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, length %u: %.*s", - inft_name_length_field, - inft_name_length_field, + ND_PRINT("\n\t\t Interface Name"); + if (inft_name_length_field % 4 != 0) { + ND_PRINT(" [length %u != N x 4]", inft_name_length_field); + nd_print_invalid(ndo); + offset += inft_name_length_field; + break; + } + if (inft_name_length_field > 64) { + ND_PRINT(" [length %u > 64]", inft_name_length_field); + nd_print_invalid(ndo); + offset += inft_name_length_field; + break; + } + ND_PRINT(", length %u", inft_name_length_field); + ND_PRINT(": %.*s", inft_name_length_field - 1, ifname_subobj->if_name); - offset += 1 + inft_name_length_field; + offset += inft_name_length_field; } if (mtu_flag) { ND_PRINT("\n\t\t MTU: %u", GET_BE_U_4(offset)); diff --git a/tests/icmp-cksum-oobr-2.out b/tests/icmp-cksum-oobr-2.out index fd2a22eb..79089e5d 100644 --- a/tests/icmp-cksum-oobr-2.out +++ b/tests/icmp-cksum-oobr-2.out @@ -7,4 +7,4 @@ This object describes the IP interface upon which a datagram arrived Interface Index: 15 IP Address sub-object: 10.10.10.10 - Interface Name, length 63: This-is-the-name-of-the-Interface-that-we-are-looking-for-[:-)] [|icmp] + Interface Name [length 63 != N x 4] (invalid) [|icmp] diff --git a/tests/icmp-rfc5837.out b/tests/icmp-rfc5837.out index e5342bc5..a65f3b7b 100644 --- a/tests/icmp-rfc5837.out +++ b/tests/icmp-rfc5837.out @@ -2,9 +2,9 @@ 10.4.0.2 > 12.4.4.4: ICMP time exceeded in-transit, length 220 IP (tos 0x0, ttl 1, id 42321, offset 0, flags [none], proto UDP (17), length 40) 12.4.4.4.42315 > 12.1.1.1.33440: UDP, length 12 - ICMP Multi-Part extension v2, checksum 0x256c (correct), length 84 + ICMP Multi-Part extension v2, checksum 0x246c (correct), length 84 Interface Identification Object (2), Class-Type: 14, length 80 This object describes the IP interface upon which a datagram arrived Interface Index: 15 IP Address sub-object: 10.10.10.10 - Interface Name, length 63: This-is-the-name-of-the-Interface-that-we-are-looking-for-[:-)] + Interface Name, length 64: This-is-the-name-of-the-Interface-that-we-are-looking-for-[:-)] diff --git a/tests/icmp-rfc5837.pcap b/tests/icmp-rfc5837.pcap index 5daa8e5a..c600bf83 100644 Binary files a/tests/icmp-rfc5837.pcap and b/tests/icmp-rfc5837.pcap differ