* [PATCH] Re: [BUG] Security Announcement: ip6tables bug - level: very high
2002-06-05 22:58 ` [BUG] Security Announcement: ip6tables bug - level: very high Andras Kis-Szabo
@ 2002-06-06 1:58 ` Andras Kis-Szabo
0 siblings, 0 replies; 2+ messages in thread
From: Andras Kis-Szabo @ 2002-06-06 1:58 UTC (permalink / raw)
To: Netfilter Devel; +Cc: Darko Kraus
[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]
Hi,
*** Description / example ***
The exapmle scenario:
# ip6tables -L
# tcpreplay6 <pure-icmpv6
# tcpreplay6 <ah-len24-spi4096-clean-dst-len8-sub2-len4-pad1-pad1
// or any other packet with extension headers
# ip6tables -A OUTPUT -p ipv6-icmp
# tcpreplay6 <pure-icmpv6
# ip6tables -L OUTPUT -n -v
Chain OUTPUT (policy ACCEPT 1 packets, 57 bytes)
pkts bytes target prot opt in out source destination
1 57 icmpv6 * * ::/0 ::/0
# tcpreplay6 <ah-len24-spi4096-clean-dst-len8-sub2-len4-pad1-pad1
// or any other packet with extension headers
*** The kernel crashed immediately. ***
(The test packets can be found in the pool:
http://www.securityaudit.hu/Netfilter/addons/TestPackets/ )
*** Background ***
The original code contains
- 2 errors in its source code and
When calculates the length of the AH header it used an index from the
'air'.
> - hdrlen = hdrptr[i] * 4 + 8;
> + hdrlen = hdrptr[1] * 4 + 8;
When set the pointer for the first header
> - hdrptr = (u_int8_t *)(ipv6 + 1);
> + hdrptr = (u_int8_t *)((void *)ipv6 + 1);
(the kernel crashed here...)
- 1 error in its design.
The code checked the extension headers - without the headers.
It set the index after the ipv6 header and started iterate the
headers until the protocol header to find the type of the
protocol. In the skbuff the ipv6 header is only the pure header,
nothing more, so it walked somewhere in the kernel memory.
*** Affected systems ***
Any Linux system, which has a rule with protocol match (-p)!
*** Preparedness ***
Any script-kiddie, or a plain user with instructions.
Automated tool: possible
*** Result ***
Kernel crash
*** Required packet ***
- any IPv6 packet with options
*** Modifications in the code ***
I got the header parser code from my extension header patches, and put
it into the ip6_tables.c.
It can be buggy - I checked with the testpackets and worked properly.
It ahndles the '-p all' counter quiestion/ticket, too.
I left the original code (with the debug messages) in the file.
For the '-p all' question:
- patkets with NONE or ESP ext. headers won't be counted (it's only a
return, so it can be changed)
- the ip6tables never adds the size of the extensions to the counters!
(It counts only the ipv6-header and the payload.)
Regards,
kisza
--
Andras Kis-Szabo Security Development, Design and Audit
-------------------------/ Zorp, NetFilter and IPv6
kisza@SecurityAudit.hu /-----Member of the BUTE-MIS-SEARCHlab------>
[-- Attachment #2: exthdr-bug-20020606 --]
[-- Type: text/plain, Size: 5696 bytes --]
--- linux-p0bug/net/ipv6/netfilter/ip6_tables.c Wed Jun 5 23:57:31 2002
+++ linux/net/ipv6/netfilter/ip6_tables.c Thu Jun 6 03:30:59 2002
@@ -7,6 +7,8 @@
* 19 Jan 2002 Harald Welte <laforge@gnumonks.org>
* - increase module usage count as soon as we have rules inside
* a table
+ * 06 Jun 2002 Andras Kis-Szabo <kisza@sch.bme.hu>
+ * - new (and real) extension header parser code
*/
#include <linux/config.h>
#include <linux/skbuff.h>
@@ -25,6 +27,7 @@
#include <linux/netfilter_ipv6/ip6_tables.h>
#define IPV6_HDR_LEN (sizeof(struct ipv6hdr))
+#define IPV6_OPTHDR_LEN (sizeof(struct ipv6_opt_hdr))
/*#define DEBUG_IP_FIREWALL*/
/*#define DEBUG_ALLOW_ALL*/ /* Useful for remote debugging */
@@ -133,12 +136,26 @@
return 0;
}
+/* Check for an extension */
+static int ipv6_ext_hdr(u8 nexthdr)
+{
+ return ( (nexthdr == IPPROTO_HOPOPTS) ||
+ (nexthdr == IPPROTO_ROUTING) ||
+ (nexthdr == IPPROTO_FRAGMENT) ||
+ (nexthdr == IPPROTO_ESP) ||
+ (nexthdr == IPPROTO_AH) ||
+ (nexthdr == IPPROTO_NONE) ||
+ (nexthdr == IPPROTO_DSTOPTS) );
+}
+
+#if 0
+/* The old extension-header parser. Sorry for the #if 0!
+ * -- kisza */
/* takes in current header and pointer to the header */
/* if another header exists, sets hdrptr to the next header
and returns the new header value, else returns 0 */
static u_int8_t ip6_nexthdr(u_int8_t currenthdr, u_int8_t *hdrptr)
{
- int i;
u_int8_t hdrlen, nexthdr = 0;
switch(currenthdr){
case IPPROTO_AH:
@@ -147,13 +164,14 @@
repeatedly...with a large stick...no, an even LARGER
stick...no, you're still not thinking big enough */
nexthdr = *hdrptr;
- hdrlen = hdrptr[i] * 4 + 8;
+ hdrlen = hdrptr[1] * 4 + 8;
hdrptr = hdrptr + hdrlen;
break;
/*stupid rfc2402 */
case IPPROTO_DSTOPTS:
case IPPROTO_ROUTING:
case IPPROTO_HOPOPTS:
+ dprintf(" Found a general header.\n");
nexthdr = *hdrptr;
hdrlen = hdrptr[1] * 8 + 8;
hdrptr = hdrptr + hdrlen;
@@ -163,13 +181,18 @@
hdrptr = hdrptr + 8;
break;
}
+
+ dprintf(" Found header: %hi\n", nexthdr);
+
return nexthdr;
}
+#endif
/* Returns whether matches rule or not. */
static inline int
-ip6_packet_match(const struct ipv6hdr *ipv6,
+ip6_packet_match(const struct sk_buff *skb,
+ const struct ipv6hdr *ipv6,
const char *indev,
const char *outdev,
const struct ip6t_ip6 *ip6info,
@@ -227,17 +250,86 @@
/* look for the desired protocol header */
if((ip6info->flags & IP6T_F_PROTO)) {
u_int8_t currenthdr = ipv6->nexthdr;
- u_int8_t *hdrptr;
- hdrptr = (u_int8_t *)(ipv6 + 1);
+ struct ipv6_opt_hdr *hdrptr;
+ u_int16_t ptr; /* Header offset in skb */
+ u_int16_t hdrlen; /* Header */
+
+ ptr = IPV6_HDR_LEN;
+
+ while (ipv6_ext_hdr(currenthdr)) {
+ /* Is there enough space for the next ext header? */
+ if (skb->len - ptr < IPV6_OPTHDR_LEN)
+ return 0;
+
+ /* NONE or ESP: there isn't protocol part */
+ /* If we want to count these packets in '-p all',
+ * we will change the return 0 to 1*/
+ if ((currenthdr == IPPROTO_NONE) ||
+ (currenthdr == IPPROTO_ESP))
+ return 0;
+
+ hdrptr = (struct ipv6_opt_hdr *)(skb->data + ptr);
+
+ /* Size calculation */
+ if (currenthdr == IPPROTO_FRAGMENT) {
+ hdrlen = 8;
+ } else if (currenthdr == IPPROTO_AH)
+ hdrlen = (hdrptr->hdrlen+2)<<2;
+ else
+ hdrlen = ipv6_optlen(hdrptr);
+
+ currenthdr = hdrptr->nexthdr;
+ ptr += hdrlen;
+ /* ptr is too large */
+ if ( ptr > skb->len )
+ return 0;
+ }
+
+ /* currenthdr contains the protocol header */
+
+ dprintf("Packet protocol %hi ?= %s%hi.\n",
+ currenthdr,
+ ip6info->invflags & IP6T_INV_PROTO ? "!":"",
+ ip6info->proto);
+
+ if (ip6info->proto == currenthdr) {
+ if(ip6info->invflags & IP6T_INV_PROTO) {
+ return 0;
+ }
+ return 1;
+ }
+
+ /*
+ * Old extension header parser - WRONG IDEA! It always
+ * causes crash with an extension!
+ * -- kisza
+ hdrptr = (u_int8_t *)((void *)ipv6 + 1);
do {
+ dprintf("Packet protocol currenthdr: %hi ?= %hi.\n",
+ currenthdr, ip6info->proto);
if (ip6info->proto == currenthdr) {
- if(ip6info->invflags & IP6T_INV_PROTO)
+ if(ip6info->invflags & IP6T_INV_PROTO) {
+ dprintf("Packet protocol %hi does not "
+ "match %hi.%s\n",
+ currenthdr, ip6info->proto,
+ ip6info->invflags &
+ IP6T_INV_PROTO ? " (INV)":"");
return 0;
+ }
+ dprintf("Packet protocol %hi "
+ "matches %hi.%s\n",
+ currenthdr, ip6info->proto,
+ ip6info->invflags &
+ IP6T_INV_PROTO ? " (INV)":"");
return 1;
}
currenthdr = ip6_nexthdr(currenthdr, hdrptr);
} while(currenthdr);
- if (!(ip6info->invflags & IP6T_INV_PROTO))
+ */
+
+ /* We need match for the '-p all', too! */
+ if ((ip6info->proto != 0) &&
+ !(ip6info->invflags & IP6T_INV_PROTO))
return 0;
}
return 1;
@@ -359,7 +451,8 @@
IP_NF_ASSERT(e);
IP_NF_ASSERT(back);
(*pskb)->nfcache |= e->nfcache;
- if (ip6_packet_match(ipv6, indev, outdev, &e->ipv6, offset)) {
+ if (ip6_packet_match(*pskb, ipv6, indev, outdev,
+ &e->ipv6, offset)) {
struct ip6t_entry_target *t;
if (IP6T_MATCH_ITERATE(e, do_match,
^ permalink raw reply [flat|nested] 2+ messages in thread