batctl: Merge bugfixes from 2024.0 1042/head
authorSven Eckelmann <sven@narfation.org>
Fri, 2 Feb 2024 21:59:34 +0000 (22:59 +0100)
committerSven Eckelmann <sven@narfation.org>
Sat, 3 Feb 2024 08:00:09 +0000 (09:00 +0100)
* 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 <sven@narfation.org>
batctl/Makefile
batctl/patches/0002-batctl-tcpdump-Fix-missing-sanity-check-for-batman-a.patch [new file with mode: 0644]
batctl/patches/0003-batctl-tcpdump-Add-missing-throughput-header-length-.patch [new file with mode: 0644]
batctl/patches/0004-batctl-tcpdump-Fix-IPv4-header-length-check.patch [new file with mode: 0644]
batctl/patches/0005-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Advert-le.patch [new file with mode: 0644]
batctl/patches/0006-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Solicit-l.patch [new file with mode: 0644]
batctl/patches/0007-batctl-tcpdump-Fix-ICMPv4-inner-IPv4-header-length-c.patch [new file with mode: 0644]

index 49ab0aa18cd4d722ffeaa96a54f3e268a4b3d96b..683f667275f96b4a84bc6607534d29c354af3fb3 100644 (file)
@@ -4,7 +4,7 @@ include $(TOPDIR)/rules.mk
 
 PKG_NAME:=batctl
 PKG_VERSION:=2022.0
-PKG_RELEASE:=3
+PKG_RELEASE:=4
 
 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/0002-batctl-tcpdump-Fix-missing-sanity-check-for-batman-a.patch b/batctl/patches/0002-batctl-tcpdump-Fix-missing-sanity-check-for-batman-a.patch
new file mode 100644 (file)
index 0000000..c297287
--- /dev/null
@@ -0,0 +1,26 @@
+From: Sven Eckelmann <sven@narfation.org>
+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 <sven@narfation.org>
+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/0003-batctl-tcpdump-Add-missing-throughput-header-length-.patch b/batctl/patches/0003-batctl-tcpdump-Add-missing-throughput-header-length-.patch
new file mode 100644 (file)
index 0000000..282341d
--- /dev/null
@@ -0,0 +1,34 @@
+From: Sven Eckelmann <sven@narfation.org>
+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 <sven@narfation.org>
+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/0004-batctl-tcpdump-Fix-IPv4-header-length-check.patch b/batctl/patches/0004-batctl-tcpdump-Fix-IPv4-header-length-check.patch
new file mode 100644 (file)
index 0000000..bb99930
--- /dev/null
@@ -0,0 +1,27 @@
+From: Sven Eckelmann <sven@narfation.org>
+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 <sven@narfation.org>
+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/0005-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Advert-le.patch b/batctl/patches/0005-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Advert-le.patch
new file mode 100644 (file)
index 0000000..4030332
--- /dev/null
@@ -0,0 +1,25 @@
+From: Sven Eckelmann <sven@narfation.org>
+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 <marco.dallato@gmail.com>
+Signed-off-by: Sven Eckelmann <sven@narfation.org>
+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/0006-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Solicit-l.patch b/batctl/patches/0006-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Solicit-l.patch
new file mode 100644 (file)
index 0000000..199e780
--- /dev/null
@@ -0,0 +1,25 @@
+From: Sven Eckelmann <sven@narfation.org>
+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 <marco.dallato@gmail.com>
+Signed-off-by: Sven Eckelmann <sven@narfation.org>
+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/0007-batctl-tcpdump-Fix-ICMPv4-inner-IPv4-header-length-c.patch b/batctl/patches/0007-batctl-tcpdump-Fix-ICMPv4-inner-IPv4-header-length-c.patch
new file mode 100644 (file)
index 0000000..0e7488e
--- /dev/null
@@ -0,0 +1,41 @@
+From: Sven Eckelmann <sven@narfation.org>
+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 <sven@narfation.org>
+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);