In message <[EMAIL PROTECTED]> you wri
te:
> 
> Hi all!
> 
> I think it's possible to hang the kernel useing isic 0.05
> (www.packetfactory.net/Projects/ISIC/), when there's a unclean match in
> iptables rules.

Thanks for the bug report.  I've just done an audit of the unclean
code: patch against 2.4.5 is below.  There were some bad thinkos
there, two fatal.

It'd be nice if someone else audits it, as I wrote it in the first
place...

Untested, please report back.

Cheers,
Rusty.
--
Premature optmztion is rt of all evl. --DK

--- linux-2.4.5-official/net/ipv4/netfilter/ipt_unclean.c       Fri Apr 28 08:43:15 
2000
+++ working-2.4.5-hotplug/net/ipv4/netfilter/ipt_unclean.c      Tue Jun 19 19:30:32 
+2001
@@ -76,7 +76,7 @@
                    = { 12, 12, ICMP_NOT_ERROR, 0, 0 } };
 
        /* Can't do anything if it's a fragment. */
-       if (!offset)
+       if (offset)
                return 1;
 
        /* Must cover type and code. */
@@ -87,7 +87,7 @@
 
        /* If not embedded. */
        if (!embedded) {
-               /* Bad checksum?  Don't print, just drop. */
+               /* Bad checksum?  Don't print, just ignore. */
                if (!more_frags
                    && ip_compute_csum((unsigned char *) icmph, datalen) != 0)
                        return 0;
@@ -108,6 +108,8 @@
                           length of iph + 8 bytes. */
                        struct iphdr *inner = (void *)icmph + 8;
 
+                       /* datalen > 8 since all ICMP_IS_ERROR types
+                           have min length > 8 */
                        if (datalen - 8 < sizeof(struct iphdr)) {
                                limpk("ICMP error internal way too short\n");
                                return 0;
@@ -155,6 +157,8 @@
                u_int32_t arg = ntohl(icmph->un.gateway);
 
                if (icmph->code == 0) {
+                       /* Code 0 means that upper 8 bits is pointer
+                           to problem. */
                        if ((arg >> 24) >= iph->ihl*4) {
                                limpk("ICMP PARAMETERPROB ptr = %u\n",
                                      ntohl(icmph->un.gateway) >> 24);
@@ -196,7 +200,7 @@
          int embedded)
 {
        /* Can't do anything if it's a fragment. */
-       if (!offset)
+       if (offset)
                return 1;
 
        /* CHECK: Must cover UDP header. */
@@ -205,7 +209,7 @@
                return 0;
        }
 
-       /* Bad checksum?  Don't print, just drop. */
+       /* Bad checksum?  Don't print, just say it's unclean. */
        /* FIXME: SRC ROUTE packets won't match checksum --RR */
        if (!more_frags && !embedded
            && csum_tcpudp_magic(iph->saddr, iph->daddr, datalen, IPPROTO_UDP,
@@ -272,7 +276,7 @@
        /* In fact, this is caught below (offset < 516). */
 
        /* Can't do anything if it's a fragment. */
-       if (!offset)
+       if (offset)
                return 1;
 
        /* CHECK: Smaller than minimal TCP hdr. */
@@ -281,7 +285,8 @@
                        limpk("Packet length %u < TCP header.\n", datalen);
                        return 0;
                }
-               /* Must have ports available (datalen >= 8). */
+               /* Must have ports available (datalen >= 8), from
+                   check_icmp which set embedded = 1 */
                /* CHECK: TCP ports inside ICMP error */
                if (!tcph->source || !tcph->dest) {
                        limpk("Zero TCP ports %u/%u.\n",
@@ -301,7 +306,7 @@
                        return 1;
        }
 
-       /* Bad checksum?  Don't print, just drop. */
+       /* Bad checksum?  Don't print, just say it's unclean. */
        /* FIXME: SRC ROUTE packets won't match checksum --RR */
        if (!more_frags && !embedded
            && csum_tcpudp_magic(iph->saddr, iph->daddr, datalen, IPPROTO_TCP,
@@ -373,6 +378,8 @@
                                      (unsigned int) opt[i], i);
                                return 0;
                        }
+                       /* Move to next option */
+                       i += opt[i+1];
                }
        }
 
@@ -442,6 +449,8 @@
                                      opt[i], i);
                                return 0;
                        }
+                       /* Move to next option */
+                       i += opt[i+1];
                }
        }
 
@@ -495,10 +504,10 @@
                return 0;
        }
 
-       /* CHECK: Min offset of frag = 128 - 60 (max IP hdr len). */
-       if (offset && offset * 8 < MIN_LIKELY_MTU - 60) {
+       /* CHECK: Min offset of frag = 128 - IP hdr len. */
+       if (offset && offset * 8 < MIN_LIKELY_MTU - iph->ihl * 4) {
                limpk("Fragment starts at %u < %u\n", offset * 8,
-                     MIN_LIKELY_MTU-60);
+                     MIN_LIKELY_MTU - iph->ihl * 4);
                return 0;
        }
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to