From: Sven Eckelmann Date: Fri, 2 Feb 2024 21:59:34 +0000 (+0100) Subject: batctl: Merge bugfixes from 2024.0 X-Git-Url: http://git.lede-project.org./?a=commitdiff_plain;h=refs%2Fpull%2F1041%2Fhead;p=feed%2Frouting.git batctl: Merge bugfixes from 2024.0 * tcpdump: Fix missing sanity check for batman-adv header * tcpdump: Add missing throughput header length check * tcpdump: Fix IPv4 header length check * tcpdump: Add missing ICMPv6 Neighbor Advert length check * tcpdump: Add missing ICMPv6 Neighbor Solicit length check * tcpdump: Fix ICMPv4 inner IPv4 header length check Signed-off-by: Sven Eckelmann --- diff --git a/batctl/Makefile b/batctl/Makefile index 70ca771..4cb336b 100644 --- a/batctl/Makefile +++ b/batctl/Makefile @@ -4,7 +4,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=batctl PKG_VERSION:=2023.1 -PKG_RELEASE:=1 +PKG_RELEASE:=2 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz PKG_SOURCE_URL:=https://downloads.open-mesh.org/batman/releases/batman-adv-$(PKG_VERSION) diff --git a/batctl/patches/0001-batctl-tcpdump-Fix-missing-sanity-check-for-batman-a.patch b/batctl/patches/0001-batctl-tcpdump-Fix-missing-sanity-check-for-batman-a.patch new file mode 100644 index 0000000..c297287 --- /dev/null +++ b/batctl/patches/0001-batctl-tcpdump-Fix-missing-sanity-check-for-batman-a.patch @@ -0,0 +1,26 @@ +From: Sven Eckelmann +Date: Sat, 27 Jan 2024 13:48:59 +0100 +Subject: batctl: tcpdump: Fix missing sanity check for batman-adv header + +parse_eth_hdr() is assuming that every ETH_P_BATMAN ethernet packet has a +valid, minimal batman-adv header (packet_type, version, ttl) attached. But +it doesn't actually check if the received buffer has enough bytes to access +the two bytes packet_type + version. So it is possible that it tries to +read outside of the received data. + +Fixes: 3bdfc388e74b ("implement simple tcpdump, first only batman packets") +Signed-off-by: Sven Eckelmann +Origin: upstream, https://git.open-mesh.org/batctl.git/commit/7ae3bdb59a7501197e12d3a7ab0d9924341e9ca8 + +--- a/tcpdump.c ++++ b/tcpdump.c +@@ -1068,6 +1068,9 @@ static void parse_eth_hdr(unsigned char + dump_vlan(packet_buff, buff_len, read_opt, time_printed); + break; + case ETH_P_BATMAN: ++ /* check for batman-adv packet_type + version */ ++ LEN_CHECK(buff_len, sizeof(*eth_hdr) + 2, "BAT HEADER") ++ + batman_ogm_packet = (struct batadv_ogm_packet *)(packet_buff + ETH_HLEN); + + if ((read_opt & COMPAT_FILTER) && diff --git a/batctl/patches/0002-batctl-tcpdump-Add-missing-throughput-header-length-.patch b/batctl/patches/0002-batctl-tcpdump-Add-missing-throughput-header-length-.patch new file mode 100644 index 0000000..282341d --- /dev/null +++ b/batctl/patches/0002-batctl-tcpdump-Add-missing-throughput-header-length-.patch @@ -0,0 +1,34 @@ +From: Sven Eckelmann +Date: Sat, 27 Jan 2024 13:49:00 +0100 +Subject: batctl: tcpdump: Add missing throughput header length check + +dump_batman_icmp() is only doing a length check for the original ICMP +packet length. But the throughput packet (which is also handled by this +function) is accessed without doing an additional length check. So it is +possible that it tries to read outside of the received data. + +Fixes: f109b3473f86 ("batctl: introduce throughput meter support") +Signed-off-by: Sven Eckelmann +Origin: upstream, https://git.open-mesh.org/batctl.git/commit/189b66496309bc1a54b4821292da2428de8ceb1c + +--- a/tcpdump.c ++++ b/tcpdump.c +@@ -863,7 +863,6 @@ static void dump_batman_icmp(unsigned ch + LEN_CHECK((size_t)buff_len - sizeof(struct ether_header), sizeof(struct batadv_icmp_packet), "BAT ICMP"); + + icmp_packet = (struct batadv_icmp_packet *)(packet_buff + sizeof(struct ether_header)); +- tp = (struct batadv_icmp_tp_packet *)icmp_packet; + + if (!time_printed) + print_time(); +@@ -894,6 +893,10 @@ static void dump_batman_icmp(unsigned ch + (size_t)buff_len - sizeof(struct ether_header)); + break; + case BATADV_TP: ++ LEN_CHECK((size_t)buff_len - sizeof(struct ether_header), sizeof(*tp), "BAT TP"); ++ ++ tp = (struct batadv_icmp_tp_packet *)icmp_packet; ++ + printf("%s: ICMP TP type %s (%hhu), id %hhu, seq %u, ttl %2d, v %d, length %zu\n", + name, tp->subtype == BATADV_TP_MSG ? "MSG" : + tp->subtype == BATADV_TP_ACK ? "ACK" : "N/A", diff --git a/batctl/patches/0003-batctl-tcpdump-Fix-IPv4-header-length-check.patch b/batctl/patches/0003-batctl-tcpdump-Fix-IPv4-header-length-check.patch new file mode 100644 index 0000000..bb99930 --- /dev/null +++ b/batctl/patches/0003-batctl-tcpdump-Fix-IPv4-header-length-check.patch @@ -0,0 +1,27 @@ +From: Sven Eckelmann +Date: Sat, 27 Jan 2024 13:49:01 +0100 +Subject: batctl: tcpdump: Fix IPv4 header length check + +dump_ip() is directly accessing the header in the header length check and +assumes that ihl can be trusted. But when when ihl is set to something less +than 5 then it would not even be possible to store the basic IPv4 header in +it. But dump_ip would have still accepted it because it didn't check if +there are at least enough bytes available to read the basic IPv4 header. 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 +Origin: upstream, https://git.open-mesh.org/batctl.git/commit/ddb254bd51aa43d216159f3be9c575369b041d35 + +--- a/tcpdump.c ++++ b/tcpdump.c +@@ -646,7 +646,9 @@ static void dump_ip(unsigned char *packe + struct icmphdr *icmphdr; + + iphdr = (struct iphdr *)packet_buff; ++ LEN_CHECK((size_t)buff_len, sizeof(*iphdr), ip_string); + LEN_CHECK((size_t)buff_len, (size_t)(iphdr->ihl * 4), ip_string); ++ LEN_CHECK((size_t)(iphdr->ihl * 4), sizeof(*iphdr), ip_string); + + if (!time_printed) + print_time(); diff --git a/batctl/patches/0004-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Advert-le.patch b/batctl/patches/0004-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Advert-le.patch new file mode 100644 index 0000000..4030332 --- /dev/null +++ b/batctl/patches/0004-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Advert-le.patch @@ -0,0 +1,25 @@ +From: Sven Eckelmann +Date: Sat, 27 Jan 2024 13:49:02 +0100 +Subject: batctl: tcpdump: Add missing ICMPv6 Neighbor Advert length check + +dump_ipv6() is doing a length check for the original ICMPv6 header length. +But the neighbor advertisement (which is also handled by this function) is +accessed without doing an additional length check. So it is possible that +it tries to read outside of the received data. + +Fixes: 35b37756f4a3 ("add IPv6 support to tcpdump parser") +Cc: Marco Dalla Torre +Signed-off-by: Sven Eckelmann +Origin: upstream, https://git.open-mesh.org/batctl.git/commit/da75747d435ca8a32a74895655a1d5bff8b7709b + +--- a/tcpdump.c ++++ b/tcpdump.c +@@ -611,6 +611,8 @@ static void dump_ipv6(unsigned char *pac + nd_nas_target, buff_len); + break; + case ND_NEIGHBOR_ADVERT: ++ LEN_CHECK((size_t)buff_len - (size_t)(sizeof(struct ip6_hdr)), ++ sizeof(*nd_advert), "ICMPv6 Neighbor Advertisement"); + nd_advert = (struct nd_neighbor_advert *)icmphdr; + inet_ntop(AF_INET6, &(nd_advert->nd_na_target), + nd_nas_target, 40); diff --git a/batctl/patches/0005-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Solicit-l.patch b/batctl/patches/0005-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Solicit-l.patch new file mode 100644 index 0000000..199e780 --- /dev/null +++ b/batctl/patches/0005-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Solicit-l.patch @@ -0,0 +1,25 @@ +From: Sven Eckelmann +Date: Sat, 27 Jan 2024 13:49:03 +0100 +Subject: batctl: tcpdump: Add missing ICMPv6 Neighbor Solicit length check + +dump_ipv6() is doing a length check for the original ICMPv6 header length. +But the neighbor solicitation (which is also handled by this function) is +accessed without doing an additional length check. So it is possible that +it tries to read outside of the received data. + +Fixes: 35b37756f4a3 ("add IPv6 support to tcpdump parser") +Cc: Marco Dalla Torre +Signed-off-by: Sven Eckelmann +Origin: upstream, https://git.open-mesh.org/batctl.git/commit/83025933cb502192d22edc89de3c57103968c4ed + +--- a/tcpdump.c ++++ b/tcpdump.c +@@ -604,6 +604,8 @@ static void dump_ipv6(unsigned char *pac + (size_t)buff_len - sizeof(struct icmp6_hdr)); + break; + case ND_NEIGHBOR_SOLICIT: ++ LEN_CHECK((size_t)buff_len - (size_t)(sizeof(struct ip6_hdr)), ++ sizeof(*nd_neigh_sol), "ICMPv6 Neighbor Solicitation"); + nd_neigh_sol = (struct nd_neighbor_solicit *)icmphdr; + inet_ntop(AF_INET6, &(nd_neigh_sol->nd_ns_target), + nd_nas_target, 40); diff --git a/batctl/patches/0006-batctl-tcpdump-Fix-ICMPv4-inner-IPv4-header-length-c.patch b/batctl/patches/0006-batctl-tcpdump-Fix-ICMPv4-inner-IPv4-header-length-c.patch new file mode 100644 index 0000000..0e7488e --- /dev/null +++ b/batctl/patches/0006-batctl-tcpdump-Fix-ICMPv4-inner-IPv4-header-length-c.patch @@ -0,0 +1,41 @@ +From: Sven Eckelmann +Date: Sat, 27 Jan 2024 13:49:04 +0100 +Subject: batctl: tcpdump: Fix ICMPv4 inner IPv4 header length check + +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 +Origin: upstream, https://git.open-mesh.org/batctl.git/commit/fb7a51466bf46a4914a32edd8e1be6ba0733cd49 + +--- a/tcpdump.c ++++ b/tcpdump.c +@@ -682,12 +682,20 @@ static void dump_ip(unsigned char *packe + (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);