Bug in Linux 2.4 / iptables MAC match module

Harald Welte laforge@gnumonks.org
Tue, 2 Oct 2001 19:57:28 +0200


--bAmEntskrkuBymla
Content-Type: multipart/mixed; boundary="jho1yZJdad60DJr+"
Content-Disposition: inline


--jho1yZJdad60DJr+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Sep 25, 2001 at 07:03:45PM +0100, Chris Wilson wrote:

> Please could you let us know as soon as you have some information
> regarding this bug. We very much hope to hear from you before Wednesday
> 3rd October 2001. If not then we shall be forced, reluctantly, to publish
> this advisory.

ok. There is news about this now.=20

As far as I understood the problem, the problematic piece of code was:

    /* Is mac pointer valid? */
    return (skb->mac.raw >=3D skb->head
	    && skb->mac.raw < skb->head + skb->len - ETH_HLEN
	    /* If so, compare... */
	    && ((memcmp(skb->mac.ethernet->h_source, info->srcaddr, ETH_ALEN)
		=3D=3D 0) ^ info->invert));

skb->head	points to first byte of the layer 2 packet
skb->mac.raw	points to first byte of destination mac address
skb->data	points to first byte of layer 3 packet (=3D=3D ip header)
skb->len	length of layer three packet (layer 2 payload) in bytes
ETH_HLEN	length of layer 2 header (14 bytes, 2*6byte mac + 2byte l3prot)

The first check checks, if the pointer to the beginning of the mac address
is greater or equal than the skb->head. That's ok.

The second check, however does something strange.  It checks if the pointer=
 to
the beginning of the mac address (first byte of destination mac) is smaller
than skb->head + skb->len - ETH_HLEN.  This doesn't seem to make sense to m=
e.

I guess the intention was to check if the whole mac address fits within the
skb's valid data area.  But this is not what was done.

skb->head + skb->len does not point to the end of the packet,
skb->data + skb->len would do.  The original calculation "head + len" leads=
 to
the assumption the packet is shorter than it really is (by "skb->data - skb=
->head" bytes short).

I think it's better to check if skb->mac.raw + ETH_HLEN <=3D skb->data. Thi=
s is
what attached patch does.

Could you please verify that your problem is gone with attached patch?

Thanks.

> Chris Wilson, NetServers lead developer.

--=20
Live long and prosper
- Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
GCS/E/IT d- s-: a-- C+++ UL++++$ P+++ L++++$ E--- W- N++ o? K- w--- O- M-=
=20
V-- PS+ PE-- Y+ PGP++ t++ 5-- !X !R tv-- b+++ DI? !D G+ e* h+ r% y+(*)

--jho1yZJdad60DJr+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ipt_mac.c.patch"
Content-Transfer-Encoding: quoted-printable

--- linux-2.4.9/net/ipv4/netfilter/ipt_mac.c	Tue Oct  2 18:50:56 2001
+++ linux-2.4.9-ipt_mac-fix/net/ipv4/netfilter/ipt_mac.c	Tue Oct  2 19:32:2=
0 2001
@@ -20,7 +20,7 @@
=20
     /* Is mac pointer valid? */
     return (skb->mac.raw >=3D skb->head
-	    && skb->mac.raw < skb->head + skb->len - ETH_HLEN
+	    && (skb->mac.raw + ETH_HLEN) <=3D skb->data
 	    /* If so, compare... */
 	    && ((memcmp(skb->mac.ethernet->h_source, info->srcaddr, ETH_ALEN)
 		=3D=3D 0) ^ info->invert));

--jho1yZJdad60DJr+--

--bAmEntskrkuBymla
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE7ugAGXaXGVTD0i/8RAuHiAJ0QJSZ9yO+tZTdM4CudfCKIKLavbQCdFksF
eiRxGr/HOVdTyG2S8G0dHEM=
=xqyD
-----END PGP SIGNATURE-----

--bAmEntskrkuBymla--