Date: Fri, 06 Apr 2001 14:35:14 -0700 From: Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca> To: freebsd-security@freebsd.org Subject: URGENT: Serious bug in IPFilter (fwd) Message-ID: <200104062135.f36LZpt67966@cwsys.cwsent.com>
next in thread | raw e-mail | index | archive | help
Should we be updating IP Filter in our source tree before 4.3-RELEASE?
This sounds like a serious bug.
Regards, Phone: (250)387-8437
Cy Schubert Fax: (250)387-5766
Team Leader, Sun/Alpha Team Internet: Cy.Schubert@osg.gov.bc.ca
Open Systems Group, ITSD, ISTA
Province of BC
------- Forwarded Message
[headers removed]
From: Darren Reed <darrenr@reed.wattle.id.au>
Message-Id: <200104061656.CAA09703@avalon.reed.wattle.id.au>
Subject: URGENT: Serious bug in IPFilter
To: ipfilter@coombs.anu.edu.au
Date: Sat, 7 Apr 2001 02:56:42 +1000 (EST)
X-Mailer: ELM [version 2.4ME+ PL37 (25)]
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary=ELM986576202-7114-0_
Content-Transfer-Encoding: 7bit
Sender: owner-ipfilter@coombs.anu.edu.au
- --ELM986576202-7114-0_
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
A *VERY* serious bug has been brought to my attention in IPFilter.
In 10 words or less, fragment caching with can let through "any" packet.
Ok, so that's 8.
Cause
=====
When matching a fragment, only srcip, dstip and IP ID# are checked and
the fragment cache is checked *before* any rules are checked. It does
not even need to be a fragment. Even if you block all fragments with
a rule, fragment cache entries can be created by packets that match
state information currently held.
How to disable fragment caching
===============================
In realtime, use adb or gdb or kgdb or whatever to change the variable
named "ipfr_inuse" to 1000000. 1000000 isn't important, it just needs
to be larger than IPFT_SIZE and an integer.
NOTE: there are no sysctl's on BSD systems to adjust this if securelevel
is > 0.
New version details with fix
============================
IP Filter 3.2.*
Email me (nobody should be using this now :*)
IP Filter 3.3.22
ftp://coombs.anu.edu.au/pub/net/ip-filter/ip_fil3.3.22.tar.gz
ftp://coombs.anu.edu.au/pub/net/ip-filter/patch-3.3.22.gz
http://coombs.anu.edu.au/~avalon/ip_fil3.3.22.tar.gz
http://coombs.anu.edu.au/~avalon/patch-3.3.22.gz
IP Filter 3.4.17
ftp://coombs.anu.edu.au/pub/net/ip-filter/ip_fil3.4.17.tar.gz
ftp://coombs.anu.edu.au/pub/net/ip-filter/patch-3.4.17.gz
http://coombs.anu.edu.au/~avalon/ip_fil3.4.17.tar.gz
http://coombs.anu.edu.au/~avalon/patch-3.4.17.gz
Frag Patches
============
One attachment each for 3.3.21 and 3.4.16. These patches do not contain
changes for NAT code to make the fragment cache selective (see below),
just stop packets which aren't meant to match from matching. You are
much better off updating the whole rev step if you can.
How to enable it in new versions
================================
Enable a security hole you say ? You will need to have
"keep state keep frags" in your rule, not just "keep state".
That is rules with just "keep state" will no longer create
fragment cache enties (as happens now).
Remaining Issues
================
1. There is an automatic frgament cache used by NAT which is now
disabled
by default and requires "frag" to be inserted into a NAT rule in order
for it to function.
2. Any and all packets which are fragments and match the required tuple
(being srcip, dstip, ipid) will be let through so long as the frag cache
entry remains.
3. Use of "keep frags" with "keep state" means fragment cache entries
can be created by packets going in *either* direction. Nothing will
get added (now) to the fragment cache without being explicitly allowed
by a rule (IPF or NAT).
Why not reassemble fragmented packets?
======================================
Because it is *really bad* for a router to do this. I run TCP/IP over a
fibre channel interface which has an MTU of 65280. I *cannot* even send
full size packets over it without them being fragmented due to buffer
size
problems so I'm not going to even think about defragmentation issues! I
don't care who does it, if you've done your networking 101, you know why
routers (i.e. firewalls) do *NOT* defragment packets.
Darren
How to exploit? Something will end up on bugtraq but so far, what I've
seen isn't a complete exploit of the problem.
- --ELM986576202-7114-0_
Content-Type: text/plain; charset=US-ASCII
Content-Disposition: attachment; filename=fragpatch-3-4-16.txt
Content-Description: fragpatch-3-4-16.txt
Content-Transfer-Encoding: 7bit
diff -cr ip_fil3.4.16/ip_frag.c ip_fil3.4.17/ip_frag.c
*** ip_fil3.4.16/ip_frag.c Mon Nov 27 21:26:56 2000
- --- ip_fil3.4.17/ip_frag.c Fri Apr 6 22:31:20 2001
***************
*** 7,13 ****
*/
#if !defined(lint)
static const char sccsid[] = "@(#)ip_frag.c 1.11 3/24/96 (C)
1993-2000 Darren Reed";
! static const char rcsid[] = "@(#)$Id: ip_frag.c,v 2.10.2.7 2000/11/27
10:26:56 darrenr Exp $";
#endif
#if defined(KERNEL) && !defined(_KERNEL)
- --- 7,13 ----
*/
#if !defined(lint)
static const char sccsid[] = "@(#)ip_frag.c 1.11 3/24/96 (C)
1993-2000 Darren Reed";
! static const char rcsid[] = "@(#)$Id: ip_frag.c,v 2.10.2.8 2001/04/06
12:31:20 darrenr Exp $";
#endif
#if defined(KERNEL) && !defined(_KERNEL)
***************
*** 141,152 ****
u_int pass;
ipfr_t *table[];
{
! ipfr_t **fp, *fra, frag;
! u_int idx;
if (ipfr_inuse >= IPFT_SIZE)
return NULL;
frag.ipfr_p = ip->ip_p;
idx = ip->ip_p;
frag.ipfr_id = ip->ip_id;
- --- 141,155 ----
u_int pass;
ipfr_t *table[];
{
! ipfr_t **fp, *fra, frag;
! u_int idx, off;
if (ipfr_inuse >= IPFT_SIZE)
return NULL;
+ if (!(fin->fin_fi.fi_fl & FI_FRAG))
+ return NULL;
+
frag.ipfr_p = ip->ip_p;
idx = ip->ip_p;
frag.ipfr_id = ip->ip_id;
***************
*** 200,206 ****
/*
* Compute the offset of the expected start of the next packet.
*/
! fra->ipfr_off = (ip->ip_off & IP_OFFMASK) + (fin->fin_dlen >> 3);
ATOMIC_INCL(ipfr_stats.ifs_new);
ATOMIC_INC32(ipfr_inuse);
return fra;
- --- 203,212 ----
/*
* Compute the offset of the expected start of the next packet.
*/
! off = ip->ip_off & IP_OFFMASK;
! if (!off)
! fra->ipfr_seen0 = 1;
! fra->ipfr_off = off + (fin->fin_dlen >> 3);
ATOMIC_INCL(ipfr_stats.ifs_new);
ATOMIC_INC32(ipfr_inuse);
return fra;
***************
*** 256,261 ****
- --- 262,270 ----
ipfr_t *f, frag;
u_int idx;
+ if (!(fin->fin_fi.fi_fl & FI_FRAG))
+ return NULL;
+
/*
* For fragments, we record protocol, packet id, TOS and both IP#'s
* (these should all be the same for all fragments of a packet).
***************
*** 283,288 ****
- --- 292,310 ----
IPFR_CMPSZ)) {
u_short atoff, off;
+ /*
+ * XXX - We really need to be guarding against the
+ * retransmission of (src,dst,id,offset-range) here
+ * because a fragmented packet is never resent with
+ * the same IP ID#.
+ */
+ off = ip->ip_off & IP_OFFMASK;
+ if (f->ipfr_seen0) {
+ if (!off || (fin->fin_fi.fi_fl & FI_SHORT))
+ continue;
+ } else if (!off)
+ f->ipfr_seen0 = 1;
+
if (f != table[idx]) {
/*
* move fragment info. to the top of the list
***************
*** 295,301 ****
f->ipfr_prev = NULL;
table[idx] = f;
}
- - off = ip->ip_off & IP_OFFMASK;
atoff = off + (fin->fin_dlen >> 3);
/*
* If we've follwed the fragments, and this is the
- --- 317,322 ----
diff -cr ip_fil3.4.16/ip_frag.h ip_fil3.4.17/ip_frag.h
*** ip_fil3.4.16/ip_frag.h Sat Nov 11 00:10:54 2000
- --- ip_fil3.4.17/ip_frag.h Fri Apr 6 22:31:20 2001
***************
*** 6,12 ****
* to the original author and the contributors.
*
* @(#)ip_frag.h 1.5 3/24/96
! * $Id: ip_frag.h,v 2.4.2.2 2000/11/10 13:10:54 darrenr Exp $
*/
#ifndef __IP_FRAG_H__
- --- 6,12 ----
* to the original author and the contributors.
*
* @(#)ip_frag.h 1.5 3/24/96
! * $Id: ip_frag.h,v 2.4.2.3 2001/04/06 12:31:20 darrenr Exp $
*/
#ifndef __IP_FRAG_H__
***************
*** 24,30 ****
u_char ipfr_p;
u_char ipfr_tos;
u_short ipfr_off;
! u_short ipfr_ttl;
frentry_t *ipfr_rule;
} ipfr_t;
- --- 24,31 ----
u_char ipfr_p;
u_char ipfr_tos;
u_short ipfr_off;
! u_char ipfr_ttl;
! u_char ipfr_seen0;
frentry_t *ipfr_rule;
} ipfr_t;
***************
*** 40,46 ****
struct ipfr **ifs_nattab;
} ipfrstat_t;
! #define IPFR_CMPSZ (4 + 4 + 2 + 1 + 1)
extern int fr_ipfrttl;
extern int fr_frag_lock;
- --- 41,48 ----
struct ipfr **ifs_nattab;
} ipfrstat_t;
! #define IPFR_CMPSZ (offsetof(ipfr_t, ipfr_off) - \
! offsetof(ipfr_t, ipfr_src))
extern int fr_ipfrttl;
extern int fr_frag_lock;
diff -cr ip_fil3.4.16/ip_state.c ip_fil3.4.17/ip_state.c
*** ip_fil3.4.16/ip_state.c Tue Jan 9 01:04:46 2001
- --- ip_fil3.4.17/ip_state.c Fri Apr 6 22:31:21 2001
***************
*** 1,5 ****
/*
! * Copyright (C) 1995-2000 by Darren Reed.
*
* Redistribution and use in source and binary forms are permitted
* provided that this notice is preserved and due credit is given
- --- 1,5 ----
/*
! * Copyright (C) 1995-2001 by Darren Reed.
*
* Redistribution and use in source and binary forms are permitted
* provided that this notice is preserved and due credit is given
***************
*** 7,13 ****
*/
#if !defined(lint)
static const char sccsid[] = "@(#)ip_state.c 1.8 6/5/96 (C) 1993-2000
Darren Reed";
! static const char rcsid[] = "@(#)$Id: ip_state.c,v 2.30.2.28
2001/01/08 14:04:46 darrenr Exp $";
#endif
#include <sys/errno.h>
- --- 7,13 ----
*/
#if !defined(lint)
static const char sccsid[] = "@(#)ip_state.c 1.8 6/5/96 (C) 1993-2000
Darren Reed";
! static const char rcsid[] = "@(#)$Id: ip_state.c,v 2.30.2.30
2001/04/06 12:31:21 darrenr Exp $";
#endif
#include <sys/errno.h>
***************
*** 688,694 ****
#endif
RWLOCK_EXIT(&ipf_state);
fin->fin_rev = IP6NEQ(is->is_dst, fin->fin_fi.fi_dst);
! if (fin->fin_fi.fi_fl & FI_FRAG)
ipfr_newfrag(ip, fin, pass ^ FR_KEEPSTATE);
return is;
}
- --- 690,696 ----
#endif
RWLOCK_EXIT(&ipf_state);
fin->fin_rev = IP6NEQ(is->is_dst, fin->fin_fi.fi_dst);
! if ((fin->fin_fi.fi_fl & FI_FRAG) && (pass & FR_KEEPFRAG))
ipfr_newfrag(ip, fin, pass ^ FR_KEEPSTATE);
return is;
}
***************
*** 1302,1307 ****
- --- 1307,1317 ----
if (!fr_tcpstate(is, fin, ip, tcp)) {
continue;
}
+ } if ((pr == IPPROTO_UDP)) {
+ if (fin->fin_rev)
+ is->is_age = fr_udpacktimeout;
+ else
+ is->is_age = fr_udptimeout;
}
break;
}
***************
*** 1345,1351 ****
fr_delstate(is);
#endif
RWLOCK_EXIT(&ipf_state);
! if (fin->fin_fi.fi_fl & FI_FRAG)
ipfr_newfrag(ip, fin, pass ^ FR_KEEPSTATE);
return fr;
}
- --- 1355,1361 ----
fr_delstate(is);
#endif
RWLOCK_EXIT(&ipf_state);
! if ((fin->fin_fi.fi_fl & FI_FRAG) && (pass & FR_KEEPFRAG))
ipfr_newfrag(ip, fin, pass ^ FR_KEEPSTATE);
return fr;
}
- --ELM986576202-7114-0_
Content-Type: text/plain; charset=US-ASCII
Content-Disposition: attachment; filename=fragpatch-3-3-21.txt
Content-Description: fragpatch-3-3-21.txt
Content-Transfer-Encoding: 7bit
diff -cr ip_fil3.3.21/ip_frag.c ip_fil3.3.22/ip_frag.c
*** ip_fil3.3.21/ip_frag.c Mon Jan 15 00:56:08 2001
- --- ip_fil3.3.22/ip_frag.c Fri Apr 6 22:31:05 2001
***************
*** 1,5 ****
/*
! * Copyright (C) 1993-1998 by Darren Reed.
*
* Redistribution and use in source and binary forms are permitted
* provided that this notice is preserved and due credit is given
- --- 1,5 ----
/*
! * Copyright (C) 1993-2001 by Darren Reed.
*
* Redistribution and use in source and binary forms are permitted
* provided that this notice is preserved and due credit is given
***************
*** 7,13 ****
*/
#if !defined(lint)
static const char sccsid[] = "@(#)ip_frag.c 1.11 3/24/96 (C)
1993-1995 Darren Reed";
! static const char rcsid[] = "@(#)$Id: ip_frag.c,v 2.4.2.7 2001/01/14
13:56:08 darrenr Exp $";
#endif
#if defined(KERNEL) && !defined(_KERNEL)
- --- 7,13 ----
*/
#if !defined(lint)
static const char sccsid[] = "@(#)ip_frag.c 1.11 3/24/96 (C)
1993-1995 Darren Reed";
! static const char rcsid[] = "@(#)$Id: ip_frag.c,v 2.4.2.8 2001/04/06
12:31:05 darrenr Exp $";
#endif
#if defined(KERNEL) && !defined(_KERNEL)
***************
*** 134,145 ****
u_int pass;
ipfr_t *table[];
{
! ipfr_t **fp, *fra, frag;
! u_int idx;
if (ipfr_inuse >= IPFT_SIZE)
return NULL;
frag.ipfr_p = ip->ip_p;
idx = ip->ip_p;
frag.ipfr_id = ip->ip_id;
- --- 134,148 ----
u_int pass;
ipfr_t *table[];
{
! ipfr_t **fp, *fra, frag;
! u_int idx, off;
if (ipfr_inuse >= IPFT_SIZE)
return NULL;
+ if (!(fin->fin_fi.fi_fl & FI_FRAG))
+ return NULL;
+
frag.ipfr_p = ip->ip_p;
idx = ip->ip_p;
frag.ipfr_id = ip->ip_id;
***************
*** 193,199 ****
/*
* Compute the offset of the expected start of the next packet.
*/
! fra->ipfr_off = (ip->ip_off & IP_OFFMASK) + (fin->fin_dlen >> 3);
ATOMIC_INC(ipfr_stats.ifs_new);
ATOMIC_INC(ipfr_inuse);
return fra;
- --- 196,205 ----
/*
* Compute the offset of the expected start of the next packet.
*/
! off = ip->ip_off & IP_OFFMASK;
! if (!off)
! fra->ipfr_seen0 = 1;
! fra->ipfr_off = off + (fin->fin_dlen >> 3);
ATOMIC_INC(ipfr_stats.ifs_new);
ATOMIC_INC(ipfr_inuse);
return fra;
***************
*** 245,250 ****
- --- 251,259 ----
ipfr_t *f, frag;
u_int idx;
+ if (!(fin->fin_fi.fi_fl & FI_FRAG))
+ return NULL;
+
/*
* For fragments, we record protocol, packet id, TOS and both IP#'s
* (these should all be the same for all fragments of a packet).
***************
*** 272,277 ****
- --- 281,299 ----
IPFR_CMPSZ)) {
u_short atoff, off;
+ /*
+ * XXX - We really need to be guarding against the
+ * retransmission of (src,dst,id,offset-range) here
+ * because a fragmented packet is never resent with
+ * the same IP ID#.
+ */
+ off = ip->ip_off & IP_OFFMASK;
+ if (f->ipfr_seen0) {
+ if (!off || (fin->fin_fi.fi_fl & FI_SHORT))
+ continue;
+ } else if (!off)
+ f->ipfr_seen0 = 1;
+
if (f != table[idx]) {
/*
* move fragment info. to the top of the list
***************
*** 284,290 ****
f->ipfr_prev = NULL;
table[idx] = f;
}
- - off = ip->ip_off & IP_OFFMASK;
atoff = off + (fin->fin_dlen >> 3);
/*
* If we've follwed the fragments, and this is the
- --- 306,311 ----
diff -cr ip_fil3.3.21/ip_frag.h ip_fil3.3.22/ip_frag.h
*** ip_fil3.3.21/ip_frag.h Sat Nov 11 00:11:45 2000
- --- ip_fil3.3.22/ip_frag.h Fri Apr 6 22:31:06 2001
***************
*** 1,12 ****
/*
! * Copyright (C) 1993-1998 by Darren Reed.
*
* Redistribution and use in source and binary forms are permitted
* provided that this notice is preserved and due credit is given
* to the original author and the contributors.
*
* @(#)ip_frag.h 1.5 3/24/96
! * $Id: ip_frag.h,v 2.2.2.1 2000/11/10 13:11:45 darrenr Exp $
*/
#ifndef __IP_FRAG_H__
- --- 1,12 ----
/*
! * Copyright (C) 1993-2001 by Darren Reed.
*
* Redistribution and use in source and binary forms are permitted
* provided that this notice is preserved and due credit is given
* to the original author and the contributors.
*
* @(#)ip_frag.h 1.5 3/24/96
! * $Id: ip_frag.h,v 2.2.2.2 2001/04/06 12:31:06 darrenr Exp $
*/
#ifndef __IP_FRAG_H__
***************
*** 24,30 ****
u_char ipfr_p;
u_char ipfr_tos;
u_short ipfr_off;
! u_short ipfr_ttl;
frentry_t *ipfr_rule;
} ipfr_t;
- --- 24,31 ----
u_char ipfr_p;
u_char ipfr_tos;
u_short ipfr_off;
! u_char ipfr_ttl;
! u_char ipfr_seen0;
frentry_t *ipfr_rule;
} ipfr_t;
***************
*** 40,46 ****
struct ipfr **ifs_nattab;
} ipfrstat_t;
! #define IPFR_CMPSZ (4 + 4 + 2 + 1 + 1)
extern int fr_ipfrttl;
extern ipfrstat_t *ipfr_fragstats __P((void));
- --- 41,48 ----
struct ipfr **ifs_nattab;
} ipfrstat_t;
! #define IPFR_CMPSZ (offsetof(ipfr_t, ipfr_off) - \
! offsetof(ipfr_t, ipfr_src))
extern int fr_ipfrttl;
extern ipfrstat_t *ipfr_fragstats __P((void));
diff -cr ip_fil3.3.21/ip_state.c ip_fil3.3.22/ip_state.c
*** ip_fil3.3.21/ip_state.c Wed Aug 9 02:00:35 2000
- --- ip_fil3.3.22/ip_state.c Fri Apr 6 22:31:07 2001
***************
*** 1,5 ****
/*
! * Copyright (C) 1995-1998 by Darren Reed.
*
* Redistribution and use in source and binary forms are permitted
* provided that this notice is preserved and due credit is given
- --- 1,5 ----
/*
! * Copyright (C) 1995-2001 by Darren Reed.
*
* Redistribution and use in source and binary forms are permitted
* provided that this notice is preserved and due credit is given
***************
*** 7,13 ****
*/
#if !defined(lint)
static const char sccsid[] = "@(#)ip_state.c 1.8 6/5/96 (C) 1993-1995
Darren Reed";
! static const char rcsid[] = "@(#)$Id: ip_state.c,v 2.3.2.28
2000/08/08 16:00:35 darrenr Exp $";
#endif
#include <sys/errno.h>
- --- 7,13 ----
*/
#if !defined(lint)
static const char sccsid[] = "@(#)ip_state.c 1.8 6/5/96 (C) 1993-1995
Darren Reed";
! static const char rcsid[] = "@(#)$Id: ip_state.c,v 2.3.2.30
2001/04/06 12:31:07 darrenr Exp $";
#endif
#include <sys/errno.h>
***************
*** 427,433 ****
#endif
RWLOCK_EXIT(&ipf_state);
fin->fin_rev = (is->is_dst.s_addr != ip->ip_dst.s_addr);
! if (fin->fin_fi.fi_fl & FI_FRAG)
ipfr_newfrag(ip, fin, pass ^ FR_KEEPSTATE);
return is;
}
- --- 427,433 ----
#endif
RWLOCK_EXIT(&ipf_state);
fin->fin_rev = (is->is_dst.s_addr != ip->ip_dst.s_addr);
! if ((fin->fin_fi.fi_fl & FI_FRAG) && (pass & FR_KEEPFRAG))
ipfr_newfrag(ip, fin, pass ^ FR_KEEPSTATE);
return is;
}
***************
*** 477,483 ****
if (!(tcp->th_flags & TH_ACK)) { /* Pretend an ack was sent */
ack = tdata->td_end;
win = 1;
! if ((tcp->th_flags == TH_SYN) && (tdata->td_maxwin == 0))
tdata->td_maxwin = 1;
} else if (((tcp->th_flags & (TH_ACK|TH_RST)) == (TH_ACK|TH_RST)) &&
(ack == 0)) {
- --- 477,484 ----
if (!(tcp->th_flags & TH_ACK)) { /* Pretend an ack was sent */
ack = tdata->td_end;
win = 1;
! if ((tcp->th_flags & TH_SYN == TH_SYN) &&
! (tdata->td_maxwin == 0))
tdata->td_maxwin = 1;
} else if (((tcp->th_flags & (TH_ACK|TH_RST)) == (TH_ACK|TH_RST)) &&
(ack == 0)) {
***************
*** 1021,1027 ****
fr_delstate(is);
#endif
RWLOCK_EXIT(&ipf_state);
! if (fin->fin_fi.fi_fl & FI_FRAG)
ipfr_newfrag(ip, fin, pass ^ FR_KEEPSTATE);
return fr;
}
- --- 1022,1028 ----
fr_delstate(is);
#endif
RWLOCK_EXIT(&ipf_state);
! if ((fin->fin_fi.fi_fl & FI_FRAG) && (pass & FR_KEEPFRAG))
ipfr_newfrag(ip, fin, pass ^ FR_KEEPSTATE);
return fr;
}
- --ELM986576202-7114-0_--
------- End of Forwarded Message
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-security" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200104062135.f36LZpt67966>