Skip to contentSkip to conversation

ip6tables: Read outside array bounds

PATCH In review Roel Kluin 2 participants 2 messages Aug 27, 2009 – Aug 31, 2009

Subsystems (3)
lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 ip6tables: Read outside array bounds

Check bounds before reading from the s6_addr array. It read 1 past the end at s6_addr[16] and eui64[] was also read 1 past the end.

---

net/ipv6/netfilter/ip6t_eui64.c +2 -2
index db610ba..7b40a20 100644
@@ -43,8 +43,8 @@ eui64_mt6(const struct sk_buff *skb, const struct xt_match_param *par)
 			eui64[0] ^= 0x02;
 
 			i = 0;
-			while (ipv6_hdr(skb)->saddr.s6_addr[8 + i] == eui64[i]
-			       && i < 8)
+			while (i < 8 && ipv6_hdr(skb)->saddr.s6_addr[8 + i] ==
+					eui64[i])
 				i++;
 
 			if (i == 8)
#1 lore raw
Quoted 8 lines · Roel Kluin

Check bounds before reading from the s6_addr array. It read 1 past the end at s6_addr[16] and eui64[] was also read 1 past the end.

net/ipv6/netfilter/ip6t_eui64.c +0 -0
index db610ba..7b40a20 100644

net/ipv6/netfilter/ip6t_eui64.c eui64_mt6(const struct sk_buff *skb, const struct xt_match_param *par) · line 50

Quoted 10 lines
net/ipv6/netfilter/ip6t_eui64.c +2 -2
@@ -43,8 +43,8 @@ eui64_mt6(const struct sk_buff *skb, const struct xt_match_param *par)
 			eui64[0] ^= 0x02;
 
 			i = 0;
-			while (ipv6_hdr(skb)->saddr.s6_addr[8 + i] == eui64[i]
-			       && i < 8)
+			while (i < 8 && ipv6_hdr(skb)->saddr.s6_addr[8 + i] ==
+					eui64[i])
 				i++;
 
 			if (i == 8)

Nice catch, thanks. We might as well use memcmp though, so I've committed this patch:

commit 488908696971c5ea1dcc5d13f29c158ba4f6ae7d

Author: Patrick McHardy <[email protected]>
Date:   Mon Aug 31 15:30:31 2009 +0200

netfilter: ip6t_eui: fix read outside array bounds

Use memcmp() instead of open coded comparison that reads one byte past the intended end.

Based on patch from Roel Kluin <[email protected]>

net/ipv6/netfilter/ip6t_eui64.c +2 -7
index db610ba..ca287f6 100644
@@ -23,7 +23,6 @@ static bool
 eui64_mt6(const struct sk_buff *skb, const struct xt_match_param *par)
 {
 	unsigned char eui64[8];
-	int i = 0;
 
 	if (!(skb_mac_header(skb) >= skb->head &&
 	      skb_mac_header(skb) + ETH_HLEN <= skb->data) &&
@@ -42,12 +41,8 @@ eui64_mt6(const struct sk_buff *skb, const struct xt_match_param *par)
 			eui64[4] = 0xfe;
 			eui64[0] ^= 0x02;
 
-			i = 0;
-			while (ipv6_hdr(skb)->saddr.s6_addr[8 + i] == eui64[i]
-			       && i < 8)
-				i++;
-
-			if (i == 8)
+			if (!memcmp(ipv6_hdr(skb)->saddr.s6_addr + 8, eui64,
+				    sizeof(eui64)))
 				return true;
 		}
 	}