-
Notifications
You must be signed in to change notification settings - Fork 377
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* 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>
- Loading branch information
Showing
7 changed files
with
179 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
26 changes: 26 additions & 0 deletions
26
batctl/patches/0001-batctl-tcpdump-Fix-missing-sanity-check-for-batman-a.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) && |
34 changes: 34 additions & 0 deletions
34
batctl/patches/0002-batctl-tcpdump-Add-missing-throughput-header-length-.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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", |
27 changes: 27 additions & 0 deletions
27
batctl/patches/0003-batctl-tcpdump-Fix-IPv4-header-length-check.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(); |
25 changes: 25 additions & 0 deletions
25
batctl/patches/0004-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Advert-le.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); |
25 changes: 25 additions & 0 deletions
25
batctl/patches/0005-batctl-tcpdump-Add-missing-ICMPv6-Neighbor-Solicit-l.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); |
41 changes: 41 additions & 0 deletions
41
batctl/patches/0006-batctl-tcpdump-Fix-ICMPv4-inner-IPv4-header-length-c.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); |