From owner-freebsd-net Sat Feb 23 3:50:53 2002 Delivered-To: freebsd-net@freebsd.org Received: from whale.sunbay.crimea.ua (whale.sunbay.crimea.ua [212.110.138.65]) by hub.freebsd.org (Postfix) with ESMTP id F098B37B405; Sat, 23 Feb 2002 03:50:36 -0800 (PST) Received: (from ru@localhost) by whale.sunbay.crimea.ua (8.11.6/8.11.2) id g1NBoX450311; Sat, 23 Feb 2002 13:50:33 +0200 (EET) (envelope-from ru) Date: Sat, 23 Feb 2002 13:50:33 +0200 From: Ruslan Ermilov To: "Crist J. Clark" Cc: net@FreeBSD.org Subject: Re: TCP Connections to a Broadcast Address Message-ID: <20020223115033.GB47437@sunbay.com> References: <20020222022626.A83807@blossom.cjclark.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020222022626.A83807@blossom.cjclark.org> User-Agent: Mutt/1.3.27i Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org On Fri, Feb 22, 2002 at 02:26:26AM -0800, Crist J. Clark wrote: > BSD-based TCP/IP code have a bug with respect to creating TCP > connections to a broadcast address. This bug can potentially be a > security vulnerability when firewall administrators assume that the > TCP implementation works correctly and does not block broadcast > addresses. > > > The Standard: > > TCP connections are only valid when the destination address is a > unicast address. That is, the destination must not be a multicast or > broadcast address. One place this is clearly specified in the > Standards is RFC 1122 (everyone's very most favorite RFC), > > 4.2.3.10 Remote Address Validation > > ... > > A TCP implementation MUST silently discard an incoming SYN > segment that is addressed to a broadcast or multicast > address. > > > The Bug: > > Uncorrected BSD-based TCP implementations do not actually check if the > destination IP address is a broadcast address. Rather, the packet's > link layer address is checked. Here is the code from FreeBSD's > tcp_input.c, > > * RFC1122 4.2.3.10, p. 104: discard bcast/mcast SYN > * in_broadcast() should never return true on a received > * packet with M_BCAST not set. > * > * Packets with a multicast source address should also > * be discarded. > */ > if (m->m_flags & (M_BCAST|M_MCAST)) > goto drop; > #ifdef INET6 > if (isipv6) { > if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst) || > IN6_IS_ADDR_MULTICAST(&ip6->ip6_src)) > goto drop; > } else > #endif > if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) || > IN_MULTICAST(ntohl(ip->ip_src.s_addr)) || > ip->ip_src.s_addr == htonl(INADDR_BROADCAST)) > goto drop; > > The comment in the code reveals the reason for the mistake. The > authors assume that no one would accidentally or maliciously break the > rules. One can easily send packets with a unicast link-layer address, > but containing an IP broadcast address. No check is made in the above > code for such a pathological situation. > Nice catch! > Adding an in_broadcast() check trivially fixes the problem. Here is a > patch (which fixes the comment too), > > Index: src/sys/netinet/tcp_input.c > =================================================================== > RCS file: /export/ncvs/src/sys/netinet/tcp_input.c,v > retrieving revision 1.146 > diff -u -r1.146 tcp_input.c > --- src/sys/netinet/tcp_input.c 4 Jan 2002 17:21:27 -0000 1.146 > +++ src/sys/netinet/tcp_input.c 17 Feb 2002 12:54:39 -0000 > @@ -798,11 +798,10 @@ > } > /* > * RFC1122 4.2.3.10, p. 104: discard bcast/mcast SYN > - * in_broadcast() should never return true on a received > - * packet with M_BCAST not set. > - * > - * Packets with a multicast source address should also > - * be discarded. > + * > + * It is possible for a malicious (or misconfigured) > + * attacker to send unicast link-layer packets with a > + * broadcast IP address. Use in_broadcast() to find them. > */ > if (m->m_flags & (M_BCAST|M_MCAST)) > goto drop; > @@ -815,7 +814,8 @@ > #endif > if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) || > IN_MULTICAST(ntohl(ip->ip_src.s_addr)) || > - ip->ip_src.s_addr == htonl(INADDR_BROADCAST)) > + ip->ip_src.s_addr == htonl(INADDR_BROADCAST) || > + in_broadcast(ip->ip_dst, m->m_pkthdr.rcvif)) > goto drop; > /* > * SYN appears to be valid; create compressed TCP state > The patch is incomplete (see dropwithreset below). Here's the tcp_input.c part of the original delta that introduced this bug: : Script started on Sat Feb 23 13:37:18 2002 : $ sccs prs -r7.35 tcp_input.c : D 7.35 93/04/07 19:28:08 sklower 159 158 00007/00003/01623 : MRs: : COMMENTS: : Mostly changes recommended by jch for variable subnets & multiple : IP addresses per physical interface. May require further work. : : $ sccs sccsdiff -up -r7.34 -r7.35 tcp_input.c : SCCS/s.tcp_input.c: 7.34 vs. 7.35 : --- /tmp/get.19874.7.34 Sat Feb 23 13:37:41 2002 : +++ /tmp/get.19874.7.35 Sat Feb 23 13:37:41 2002 : @@ -518,9 +518,13 @@ findpcb: : goto dropwithreset; : if ((tiflags & TH_SYN) == 0) : goto drop; : - /* RFC1122 4.2.3.10, p. 104: discard bcast/mcast SYN */ : + /* : + * RFC1122 4.2.3.10, p. 104: discard bcast/mcast SYN : + * in_broadcast() should never return true on a received : + * packet with M_BCAST not set. : + */ : if (m->m_flags & (M_BCAST|M_MCAST) || : - in_broadcast(ti->ti_dst) || IN_MULTICAST(ti->ti_dst.s_addr)) : + IN_MULTICAST(ti->ti_dst.s_addr)) : goto drop; : am = m_get(M_DONTWAIT, MT_SONAME); /* XXX */ : if (am == NULL) : @@ -1271,7 +1275,7 @@ dropwithreset: : * Don't bother to respond if destination was broadcast/multicast. : */ : if ((tiflags & TH_RST) || m->m_flags & (M_BCAST|M_MCAST) || : - in_broadcast(ti->ti_dst) || IN_MULTICAST(ti->ti_dst.s_addr)) : + IN_MULTICAST(ti->ti_dst.s_addr)) : goto drop; : if (tiflags & TH_ACK) : tcp_respond(tp, ti, m, (tcp_seq)0, ti->ti_ack, TH_RST); : $ : Script done on Sat Feb 23 13:37:43 2002 I think you should just back the CSRG revision 7.35 out of tcp_input.c, mentioning what was wrong with removing in_broadcast() check. route add -net 192.168.4 192.168.1.1 ping 192.168.4.255 on a directly attached 192.168.1 network isn't a "malicious use". Cheers, -- Ruslan Ermilov Sysadmin and DBA, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message