dump_ip() is doing a length check for the inner (inside ICMP) IPv4 header length. But it is just assuming that the inner ICMPv4 header has ihl set to 5 - without actually checking for this. The more complex IPv4 header length check for the outer IPv4 header is missing before it tries to access the UDP header using the inner ihl IPv4 header length information. So it is possible that it tries to read outside of the received data.
Fixes: 75d68356f3fa ("[batctl] tcpdump - add basic IPv4 support") Signed-off-by: Sven Eckelmann sven@narfation.org --- tcpdump.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/tcpdump.c b/tcpdump.c index c253755..c6aca27 100644 --- a/tcpdump.c +++ b/tcpdump.c @@ -730,12 +730,20 @@ static void dump_ip(unsigned char *packet_buff, ssize_t buff_len, (size_t)buff_len - (iphdr->ihl * 4)); break; case ICMP_DEST_UNREACH: - LEN_CHECK((size_t)buff_len - (iphdr->ihl * 4) - sizeof(struct icmphdr), - sizeof(struct iphdr) + 8, "ICMP DEST_UNREACH"); - switch (icmphdr->code) { case ICMP_PORT_UNREACH: + LEN_CHECK((size_t)buff_len - (iphdr->ihl * 4) - sizeof(struct icmphdr), + sizeof(struct iphdr), "ICMP DEST_UNREACH"); + + /* validate inner IP header information */ tmp_iphdr = (struct iphdr *)(((char *)icmphdr) + sizeof(struct icmphdr)); + LEN_CHECK((size_t)buff_len - (iphdr->ihl * 4) - sizeof(struct icmphdr), + (size_t)(tmp_iphdr->ihl * 4), "ICMP DEST_UNREACH"); + LEN_CHECK((size_t)(tmp_iphdr->ihl * 4), sizeof(*iphdr), "ICMP DEST_UNREACH"); + + LEN_CHECK((size_t)buff_len - (iphdr->ihl * 4) - sizeof(struct icmphdr) - (tmp_iphdr->ihl * 4), + sizeof(*tmp_udphdr), "ICMP DEST_UNREACH"); + tmp_udphdr = (struct udphdr *)(((char *)tmp_iphdr) + (tmp_iphdr->ihl * 4));
printf("%s: ICMP ", ipdst);