[PATCH]: invaild TCP/UDP matching when ipv6 extension header exists

[PATCH]: invaild TCP/UDP matching when ipv6 extension header exists

[Yasuyuki Kozakai]

Hi,

tcp_match() and udp_match() in ip6tables.c assume that previous header
of TCP/UDP header is IPv6 Header. So, for example, 1st of fragmented UDP
packet, AHed packets can't correctly match the rules which use
"--sport" and so on.

This patch use ipv6_skip_exthdr() . But this function has the bug which
access invalid memory area when found Fragment Header.
So this patch includes the change for that, too.

Regards,

----------------------------------------
Yasuyuki KOZAKAI

Communication Platform Laboratory,
Corporate Research & Development Center,
Toshiba Corporation

yasuyuki.kozakai@toshiba.co.jp
----------------------------------------

Subject: (usagi-users 02784) about fragment (ip6tables)
From: ques_hiro@hotmail.com
Date: Tue, 27 Jan 2004 01:12:45 +0000

> Hello Users Group,
> 
> >From contents which are asked here, although it may not be, I need your 
> help well.
> 
> There is a question about the filtering method of fragment pachet using 
> ip6tables.
> 
> A and B prepare two PCs, the command of ip6tables of the following [ B ] is 
> 
> struck, 
> and 4000 bytes of UDP packet (address port number 1025) is sent from A to 
> B.
> 
> (PC_B) %ip6tables -A INPUT -p udp -m frag --dport 1025 -j DROP
> 
> In this environment, since MTU was 1500 bytes, the packet was 
> fragmentation-ized by  three, and all the packets reached and 
> carried out to B.
> 
> Then, although it is a question, since there is information on a port 
> number in the 
> first fragmentation-ized packet, although it thinks that it is filtered and 
> 
> two of  succession pass, why is it?
> 
> Is it the specification which cannot use the frag option and the port 
> option together?
> 
> If there is a person who knows, I will advice-wish-do.
> 
> _________________________________________________________________

Re: [PATCH]: invaild TCP/UDP matching when ipv6 extension header exists

[Yasuyuki Kozakai]

[-- Attachment #1: Type: Text/Plain, Size: 585 bytes --]


Sorry, I forgot to send the patch.

From: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>
Date: Sat, 31 Jan 2004 15:49:32 +0900 (JST)

> 
> Hi,
> 
> tcp_match() and udp_match() in ip6tables.c assume that previous header
> of TCP/UDP header is IPv6 Header. So, for example, 1st of fragmented UDP
> packet, AHed packets can't correctly match the rules which use
> "--sport" and so on.
> 
> This patch use ipv6_skip_exthdr() . But this function has the bug which
> access invalid memory area when found Fragment Header.
> So this patch includes the change for that, too.
> 
> Regards,

[-- Attachment #2: tcp_udp_match.patch --]
[-- Type: Text/Plain, Size: 6460 bytes --]

diff -Nur linux-2.6.1/net/ipv6/exthdrs.c linux-2.6.1-fixed/net/ipv6/exthdrs.c
--- linux-2.6.1/net/ipv6/exthdrs.c	2004-01-09 15:59:06.000000000 +0900
+++ linux-2.6.1-fixed/net/ipv6/exthdrs.c	2004-01-31 01:27:38.000000000 +0900
@@ -738,8 +738,16 @@
 		if (skb_copy_bits(skb, start, &hdr, sizeof(hdr)))
 			BUG();
 		if (nexthdr == NEXTHDR_FRAGMENT) {
-			struct frag_hdr *fhdr = (struct frag_hdr *) &hdr;
-			if (ntohs(fhdr->frag_off) & ~0x7)
+			unsigned short frag_off;
+			if (skb_copy_bits(skb,
+					  start+offsetof(struct frag_hdr,
+							 frag_off),
+					  &frag_off,
+					  sizeof(frag_off))) {
+				return -1;
+			}
+
+			if (ntohs(frag_off) & ~0x7)
 				break;
 			hdrlen = 8;
 		} else if (nexthdr == NEXTHDR_AUTH)
diff -Nur linux-2.6.1/net/ipv6/ipv6_syms.c linux-2.6.1-fixed/net/ipv6/ipv6_syms.c
--- linux-2.6.1/net/ipv6/ipv6_syms.c	2004-01-09 15:59:47.000000000 +0900
+++ linux-2.6.1-fixed/net/ipv6/ipv6_syms.c	2004-01-31 01:41:41.000000000 +0900
@@ -46,3 +46,4 @@
 EXPORT_SYMBOL(ip6_flush_pending_frames);
 EXPORT_SYMBOL(ip6_push_pending_frames);
 EXPORT_SYMBOL(ipv6_push_nfrag_opts);
+EXPORT_SYMBOL(ipv6_skip_exthdr);
diff -Nur linux-2.6.1/net/ipv6/netfilter/ip6_tables.c linux-2.6.1-fixed/net/ipv6/netfilter/ip6_tables.c
--- linux-2.6.1/net/ipv6/netfilter/ip6_tables.c	2004-01-09 16:00:02.000000000 +0900
+++ linux-2.6.1-fixed/net/ipv6/netfilter/ip6_tables.c	2004-01-31 01:27:56.000000000 +0900
@@ -1531,22 +1531,24 @@
 
 static int
 tcp_find_option(u_int8_t option,
-		const struct tcphdr *tcp,
-		u_int16_t datalen,
+		const struct sk_buff *skb,
+		unsigned int optoff,
+		unsigned int optlen,
 		int invert,
 		int *hotdrop)
 {
-	unsigned int i = sizeof(struct tcphdr);
-	const u_int8_t *opt = (u_int8_t *)tcp;
+	/* tcp.doff is only 4 bits, ie. max 15 * 4 bytes */
+	char opt[60 - sizeof(struct tcphdr)];
+	unsigned int i;
 
 	duprintf("tcp_match: finding option\n");
 	/* If we don't have the whole header, drop packet. */
-	if (tcp->doff * 4 > datalen) {
+	if (skb_copy_bits(skb, optoff, opt, optlen) < 0) {
 		*hotdrop = 1;
 		return 0;
 	}
 
-	while (i < tcp->doff * 4) {
+	for (i = 0; i < optlen; ) {
 		if (opt[i] == option) return !invert;
 		if (opt[i] < 2) i++;
 		else i += opt[i+1]?:1;
@@ -1565,21 +1567,34 @@
 	  u_int16_t datalen,
 	  int *hotdrop)
 {
-	const struct tcphdr *tcp = hdr;
+	struct tcphdr tcph;
 	const struct ip6t_tcp *tcpinfo = matchinfo;
+	int tcpoff;
+	u8 nexthdr = skb->nh.ipv6h->nexthdr;
 
-	/* To quote Alan:
+	if (offset) {
+		/* To quote Alan:
 
-	   Don't allow a fragment of TCP 8 bytes in. Nobody normal
-	   causes this. Its a cracker trying to break in by doing a
-	   flag overwrite to pass the direction checks.
-	*/
+		   Don't allow a fragment of TCP 8 bytes in. Nobody normal
+		   causes this. Its a cracker trying to break in by doing a
+		   flag overwrite to pass the direction checks.
+		*/
+		if (offset == 1) {
+			duprintf("Dropping evil TCP offset=1 frag.\n");
+			*hotdrop = 1;
+		}
+		/* Must not be a fragment. */
+		return 0;
+	}
 
-	if (offset == 1) {
-		duprintf("Dropping evil TCP offset=1 frag.\n");
-		*hotdrop = 1;
+	tcpoff = (u8*)(skb->nh.ipv6h+1) - skb->data;
+	tcpoff = ipv6_skip_exthdr(skb, tcpoff, &nexthdr, skb->len - tcpoff);
+	if (tcpoff < 0 || tcpoff > skb->len || nexthdr != IPPROTO_TCP)
 		return 0;
-	} else if (offset == 0 && datalen < sizeof(struct tcphdr)) {
+
+#define FWINVTCP(bool,invflg) ((bool) ^ !!(tcpinfo->invflags & invflg))
+
+	if (skb_copy_bits(skb, tcpoff, &tcph, sizeof(tcph)) < 0) {
 		/* We've been asked to examine this packet, and we
 		   can't.  Hence, no choice but to drop. */
 		duprintf("Dropping evil TCP offset=0 tinygram.\n");
@@ -1587,27 +1602,26 @@
 		return 0;
 	}
 
-	/* FIXME: Try tcp doff >> packet len against various stacks --RR */
-
-#define FWINVTCP(bool,invflg) ((bool) ^ !!(tcpinfo->invflags & invflg))
-
-	/* Must not be a fragment. */
-	return !offset
-		&& port_match(tcpinfo->spts[0], tcpinfo->spts[1],
-			      ntohs(tcp->source),
-			      !!(tcpinfo->invflags & IP6T_TCP_INV_SRCPT))
-		&& port_match(tcpinfo->dpts[0], tcpinfo->dpts[1],
-			      ntohs(tcp->dest),
-			      !!(tcpinfo->invflags & IP6T_TCP_INV_DSTPT))
-		&& FWINVTCP((((unsigned char *)tcp)[13]
-			     & tcpinfo->flg_mask)
-			    == tcpinfo->flg_cmp,
-			    IP6T_TCP_INV_FLAGS)
-		&& (!tcpinfo->option
-		    || tcp_find_option(tcpinfo->option, tcp, datalen,
-				       tcpinfo->invflags
-				       & IP6T_TCP_INV_OPTION,
-				       hotdrop));
+	if (!port_match(tcpinfo->spts[0], tcpinfo->spts[1],
+			ntohs(tcph.source),
+			!!(tcpinfo->invflags & IP6T_TCP_INV_SRCPT)))
+		return 0;
+	if (!port_match(tcpinfo->dpts[0], tcpinfo->dpts[1],
+			ntohs(tcph.dest),
+			!!(tcpinfo->invflags & IP6T_TCP_INV_DSTPT)))
+		return 0;
+	if (!FWINVTCP((((unsigned char *)&tcph)[13] & tcpinfo->flg_mask)
+		      == tcpinfo->flg_cmp,
+		      IP6T_TCP_INV_FLAGS))
+		return 0;
+	if (tcpinfo->option &&
+	    !tcp_find_option(tcpinfo->option, skb,
+			     tcpoff + sizeof(tcph),
+			     tcph.doff*4 - sizeof(tcph),
+			     tcpinfo->invflags & IP6T_TCP_INV_OPTION,
+			     hotdrop))
+		return 0;
+	return 1;
 }
 
 /* Called when user tries to insert an entry of this type. */
@@ -1637,10 +1651,21 @@
 	  u_int16_t datalen,
 	  int *hotdrop)
 {
-	const struct udphdr *udp = hdr;
+	struct udphdr udph;
 	const struct ip6t_udp *udpinfo = matchinfo;
+	int udpoff;
+	u8 nexthdr = skb->nh.ipv6h->nexthdr;
+
+	/* Must not be a fragment. */
+	if (offset)
+		return 0;
+
+	udpoff = (u8*)(skb->nh.ipv6h+1) - skb->data;
+	udpoff = ipv6_skip_exthdr(skb, udpoff, &nexthdr, skb->len - udpoff);
+	if (udpoff < 0 || udpoff > skb->len || nexthdr != IPPROTO_UDP)
+		return 0;
 
-	if (offset == 0 && datalen < sizeof(struct udphdr)) {
+	if (skb_copy_bits(skb, udpoff, &udph, sizeof(udph)) < 0) {
 		/* We've been asked to examine this packet, and we
 		   can't.  Hence, no choice but to drop. */
 		duprintf("Dropping evil UDP tinygram.\n");
@@ -1648,13 +1673,11 @@
 		return 0;
 	}
 
-	/* Must not be a fragment. */
-	return !offset
-		&& port_match(udpinfo->spts[0], udpinfo->spts[1],
-			      ntohs(udp->source),
-			      !!(udpinfo->invflags & IP6T_UDP_INV_SRCPT))
+	return port_match(udpinfo->spts[0], udpinfo->spts[1],
+			  ntohs(udph.source),
+			  !!(udpinfo->invflags & IP6T_UDP_INV_SRCPT))
 		&& port_match(udpinfo->dpts[0], udpinfo->dpts[1],
-			      ntohs(udp->dest),
+			      ntohs(udph.dest),
 			      !!(udpinfo->invflags & IP6T_UDP_INV_DSTPT));
 }
 

Re: [PATCH]: invaild TCP/UDP matching when ipv6 extension header exists

[Yasuyuki Kozakai]

[-- Attachment #1: Type: Text/Plain, Size: 1071 bytes --]


Hello,

I sent the patch which fixes this bug to netfilter-devel, but it include
other bug... sorry, I rewrite patch for ip6_tables.c .

Please don't forget apply the patch which fixes the bug in ipv6_skip_exthdr()
before you tests this patch. I sent it last few minutes to netdev and
netfilter-devel.

Regards,

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>


From: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>
Subject: [PATCH]: invaild TCP/UDP matching when ipv6 extension header exists
Date: Sat, 31 Jan 2004 15:49:32 +0900 (JST)


> Hi,
> 
> tcp_match() and udp_match() in ip6tables.c assume that previous header
> of TCP/UDP header is IPv6 Header. So, for example, 1st of fragmented UDP
> packet, AHed packets can't correctly match the rules which use
> "--sport" and so on.
> 
> This patch use ipv6_skip_exthdr() . But this function has the bug which
> access invalid memory area when found Fragment Header.
> So this patch includes the change for that, too.
> 
> Regards,

[-- Attachment #2: tcp-udp.patch --]
[-- Type: Text/Plain, Size: 2711 bytes --]

diff -Nur linux-2.6.3/net/ipv6/ipv6_syms.c linux-2.6.3-fixed/net/ipv6/ipv6_syms.c
--- linux-2.6.3/net/ipv6/ipv6_syms.c	2004-02-18 12:58:48.000000000 +0900
+++ linux-2.6.3-fixed/net/ipv6/ipv6_syms.c	2004-02-19 19:11:12.000000000 +0900
@@ -46,3 +46,4 @@
 EXPORT_SYMBOL(ip6_flush_pending_frames);
 EXPORT_SYMBOL(ip6_push_pending_frames);
 EXPORT_SYMBOL(ipv6_push_nfrag_opts);
+EXPORT_SYMBOL(ipv6_skip_exthdr);
diff -Nur linux-2.6.3/net/ipv6/netfilter/ip6_tables.c linux-2.6.3-fixed/net/ipv6/netfilter/ip6_tables.c
--- linux-2.6.3/net/ipv6/netfilter/ip6_tables.c	2004-02-18 12:59:22.000000000 +0900
+++ linux-2.6.3-fixed/net/ipv6/netfilter/ip6_tables.c	2004-02-20 12:39:48.155754688 +0900
@@ -1570,8 +1570,10 @@
 	  u_int16_t datalen,
 	  int *hotdrop)
 {
-	const struct tcphdr *tcp = hdr;
+	const struct tcphdr *tcp;
 	const struct ip6t_tcp *tcpinfo = matchinfo;
+	int tcpoff;
+	u8 nexthdr = skb->nh.ipv6h->nexthdr;
 
 	/* To quote Alan:
 
@@ -1592,6 +1594,24 @@
 		return 0;
 	}
 
+	tcpoff = (u8*)(skb->nh.ipv6h + 1) - skb->data;
+	tcpoff = ipv6_skip_exthdr(skb, tcpoff, &nexthdr, skb->len - tcpoff);
+	if (tcpoff < 0 || tcpoff > skb->len) {
+		duprintf("tcp_match: cannot skip exthdr. Dropping.\n");
+		*hotdrop = 1;
+		return 0;
+	} else if (nexthdr == IPPROTO_FRAGMENT)
+		return 0;
+	else if (nexthdr != IPPROTO_TCP ||
+		 skb->len - tcpoff < sizeof(struct tcphdr)) {
+		/* cannot be occured */
+		duprintf("tcp_match: cannot get TCP header. Dropping.\n");
+		*hotdrop = 1;
+		return 0;
+	}
+
+	tcp = (struct tcphdr *)(skb->data + tcpoff);
+
 	/* FIXME: Try tcp doff >> packet len against various stacks --RR */
 
 #define FWINVTCP(bool,invflg) ((bool) ^ !!(tcpinfo->invflags & invflg))
@@ -1642,8 +1662,10 @@
 	  u_int16_t datalen,
 	  int *hotdrop)
 {
-	const struct udphdr *udp = hdr;
+	const struct udphdr *udp;
 	const struct ip6t_udp *udpinfo = matchinfo;
+	int udpoff;
+	u8 nexthdr = skb->nh.ipv6h->nexthdr;
 
 	if (offset == 0 && datalen < sizeof(struct udphdr)) {
 		/* We've been asked to examine this packet, and we
@@ -1653,6 +1675,23 @@
 		return 0;
 	}
 
+	udpoff = (u8*)(skb->nh.ipv6h + 1) - skb->data;
+	udpoff = ipv6_skip_exthdr(skb, udpoff, &nexthdr, skb->len - udpoff);
+	if (udpoff < 0 || udpoff > skb->len) {
+		duprintf("udp_match: cannot skip exthdr. Dropping.\n");
+		*hotdrop = 1;
+		return 0;
+	} else if (nexthdr == IPPROTO_FRAGMENT)
+		return 0;
+	else if (nexthdr != IPPROTO_UDP ||
+		 skb->len - udpoff < sizeof(struct udphdr)) {
+		duprintf("udp_match: cannot get UDP header. Dropping.\n");
+		*hotdrop = 1;
+		return 0;
+	}
+
+	udp = (struct udphdr *)(skb->data + udpoff);
+
 	/* Must not be a fragment. */
 	return !offset
 		&& port_match(udpinfo->spts[0], udpinfo->spts[1],

Re: [PATCH]: invaild TCP/UDP matching when ipv6 extension header exists

[David S. Miller]

On Fri, 20 Feb 2004 15:12:17 +0900 (JST)
Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> wrote:

> I sent the patch which fixes this bug to netfilter-devel, but it include
> other bug... sorry, I rewrite patch for ip6_tables.c .
> 
> Please don't forget apply the patch which fixes the bug in ipv6_skip_exthdr()
> before you tests this patch. I sent it last few minutes to netdev and
> netfilter-devel.

I have applied this patch too, thanks a lot.

Re: [PATCH]: invaild TCP/UDP matching when ipv6 extension header exists

[Yasuyuki Kozakai]

[-- Attachment #1: Type: Text/Plain, Size: 973 bytes --]


Hi,

This patch is for linux 2.4.26-pre1 .

Summery:
tcp_match() and udp_match() in ip6tables.c assume that previous header
of TCP/UDP header is IPv6 Header. So, for example, 1st of fragmented UDP
packet, AHed packets can't correctly match the rules which use
"--sport" and so on.

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>


From: "David S. Miller" <davem@redhat.com>
Date: Fri, 20 Feb 2004 09:31:58 -0800

> On Fri, 20 Feb 2004 15:12:17 +0900 (JST)
> Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> wrote:
> 
> > I sent the patch which fixes this bug to netfilter-devel, but it include
> > other bug... sorry, I rewrite patch for ip6_tables.c .
> > 
> > Please don't forget apply the patch which fixes the bug in ipv6_skip_exthdr()
> > before you tests this patch. I sent it last few minutes to netdev and
> > netfilter-devel.
> 
> I have applied this patch too, thanks a lot.


[-- Attachment #2: linux-2.4.26-pre1-tcpudp.patch --]
[-- Type: Text/Plain, Size: 2743 bytes --]

diff -Nur linux-2.4.26-pre1/net/ipv6/ipv6_syms.c linux-2.4.26-pre1-fixed/net/ipv6/ipv6_syms.c
--- linux-2.4.26-pre1/net/ipv6/ipv6_syms.c	2003-11-29 03:26:21.000000000 +0900
+++ linux-2.4.26-pre1-fixed/net/ipv6/ipv6_syms.c	2004-02-26 11:03:19.000000000 +0900
@@ -33,3 +33,5 @@
 EXPORT_SYMBOL(ipv6_get_saddr);
 EXPORT_SYMBOL(ipv6_chk_addr);
 EXPORT_SYMBOL(in6_dev_finish_destroy);
+EXPORT_SYMBOL(ipv6_skip_exthdr);
+
diff -Nur linux-2.4.26-pre1/net/ipv6/netfilter/ip6_tables.c linux-2.4.26-pre1-fixed/net/ipv6/netfilter/ip6_tables.c
--- linux-2.4.26-pre1/net/ipv6/netfilter/ip6_tables.c	2004-02-18 22:36:32.000000000 +0900
+++ linux-2.4.26-pre1-fixed/net/ipv6/netfilter/ip6_tables.c	2004-02-26 10:45:26.000000000 +0900
@@ -1568,8 +1568,10 @@
 	  u_int16_t datalen,
 	  int *hotdrop)
 {
-	const struct tcphdr *tcp = hdr;
+	const struct tcphdr *tcp;
 	const struct ip6t_tcp *tcpinfo = matchinfo;
+	int tcpoff;
+	u8 nexthdr = skb->nh.ipv6h->nexthdr;
 
 	/* To quote Alan:
 
@@ -1590,6 +1592,24 @@
 		return 0;
 	}
 
+	tcpoff = (u8*)(skb->nh.ipv6h + 1) - skb->data;
+	tcpoff = ipv6_skip_exthdr(skb, tcpoff, &nexthdr, skb->len - tcpoff);
+	if (tcpoff < 0 || tcpoff > skb->len) {
+		duprintf("tcp_match: cannot skip exthdr. Dropping.\n");
+		*hotdrop = 1;
+		return 0;
+	} else if (nexthdr == IPPROTO_FRAGMENT)
+		return 0;
+	else if (nexthdr != IPPROTO_TCP ||
+		 skb->len - tcpoff < sizeof(struct tcphdr)) {
+		/* cannot be occured */
+		duprintf("tcp_match: cannot get TCP header. Dropping.\n");
+		*hotdrop = 1;
+		return 0;
+	}
+
+	tcp = (struct tcphdr *)(skb->data + tcpoff);
+
 	/* FIXME: Try tcp doff >> packet len against various stacks --RR */
 
 #define FWINVTCP(bool,invflg) ((bool) ^ !!(tcpinfo->invflags & invflg))
@@ -1640,8 +1660,10 @@
 	  u_int16_t datalen,
 	  int *hotdrop)
 {
-	const struct udphdr *udp = hdr;
+	const struct udphdr *udp;
 	const struct ip6t_udp *udpinfo = matchinfo;
+	int udpoff;
+	u8 nexthdr = skb->nh.ipv6h->nexthdr;
 
 	if (offset == 0 && datalen < sizeof(struct udphdr)) {
 		/* We've been asked to examine this packet, and we
@@ -1651,6 +1673,23 @@
 		return 0;
 	}
 
+	udpoff = (u8*)(skb->nh.ipv6h + 1) - skb->data;
+	udpoff = ipv6_skip_exthdr(skb, udpoff, &nexthdr, skb->len - udpoff);
+	if (udpoff < 0 || udpoff > skb->len) {
+		duprintf("udp_match: cannot skip exthdr. Dropping.\n");
+		*hotdrop = 1;
+		return 0;
+	} else if (nexthdr == IPPROTO_FRAGMENT)
+		return 0;
+	else if (nexthdr != IPPROTO_UDP ||
+		 skb->len - udpoff < sizeof(struct udphdr)) {
+		duprintf("udp_match: cannot get UDP header. Dropping.\n");
+		*hotdrop = 1;
+		return 0;
+	}
+
+	udp = (struct udphdr *)(skb->data + udpoff);
+
 	/* Must not be a fragment. */
 	return !offset
 		&& port_match(udpinfo->spts[0], udpinfo->spts[1],

Re: [PATCH]: invaild TCP/UDP matching when ipv6 extension header exists

[David S. Miller]

On Thu, 26 Feb 2004 13:05:50 +0900 (JST)
Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> wrote:

> This patch is for linux 2.4.26-pre1 .
> 
> Summery:
> tcp_match() and udp_match() in ip6tables.c assume that previous header
> of TCP/UDP header is IPv6 Header. So, for example, 1st of fragmented UDP
> packet, AHed packets can't correctly match the rules which use
> "--sport" and so on.

Also applied, thank you.