From owner-freebsd-security Fri Apr 6 14:37: 3 2001 Delivered-To: freebsd-security@freebsd.org Received: from point.osg.gov.bc.ca (point.osg.gov.bc.ca [142.32.102.44]) by hub.freebsd.org (Postfix) with ESMTP id 751F237B424 for ; Fri, 6 Apr 2001 14:36:41 -0700 (PDT) (envelope-from Cy.Schubert@uumail.gov.bc.ca) Received: (from daemon@localhost) by point.osg.gov.bc.ca (8.8.7/8.8.8) id OAA17925 for ; Fri, 6 Apr 2001 14:36:40 -0700 Received: from passer.osg.gov.bc.ca(142.32.110.29) via SMTP by point.osg.gov.bc.ca, id smtpda17923; Fri Apr 6 14:36:29 2001 Received: (from uucp@localhost) by passer.osg.gov.bc.ca (8.11.2/8.9.1) id f36LaOW65049 for ; Fri, 6 Apr 2001 14:36:24 -0700 (PDT) Received: from cwsys9.cwsent.com(10.2.2.1), claiming to be "cwsys.cwsent.com" via SMTP by passer9.cwsent.com, id smtpdh65047; Fri Apr 6 14:35:52 2001 Received: (from uucp@localhost) by cwsys.cwsent.com (8.11.3/8.9.1) id f36LZpt67966 for ; Fri, 6 Apr 2001 14:35:51 -0700 (PDT) Message-Id: <200104062135.f36LZpt67966@cwsys.cwsent.com> Received: from localhost.cwsent.com(127.0.0.1), claiming to be "cwsys" via SMTP by localhost.cwsent.com, id smtpdh67960; Fri Apr 6 14:35:15 2001 X-Mailer: exmh version 2.3.1 01/18/2001 with nmh-1.0.4 Reply-To: Cy Schubert - ITSD Open Systems Group From: Cy Schubert - ITSD Open Systems Group X-Sender: schubert To: freebsd-security@freebsd.org Subject: URGENT: Serious bug in IPFilter (fwd) Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Fri, 06 Apr 2001 14:35:14 -0700 Sender: owner-freebsd-security@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org 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 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 - --- 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 *************** *** 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 - --- 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 *************** *** 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