Re: Found a dead-freeze bug, in xt_ipv4options.c -- patch provided

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday 2011-11-03 17:28, Eivind Naess wrote:
>-static uint32_t ipv4options_rd(const uint8_t *data, int len)
>+static uint32_t ipv4options_rd(const uint8_t *ptr, int len)
> {
>     uint32_t opts = 0;
>+    uint32_t olen = 0;
> 
>     while (len >= 2) {
>-        opts |= 1 << (data[0] & 0x1F);
>-        len  -= data[1];
>-        data += data[1];
>+    
>+        /* Handle special options */
>+        switch (*ptr) {
>+        case IPOPT_END:
>+            return opts;
>+        case IPOPT_NOOP:
>+            len--;
>+            ptr++;
>+            continue;
>+        }
>+        
>+        /* Get the length of the option */
>+        olen = ptr[1];
>+        if (olen < 2 || olen > len)
>+            return opts;
>+        
>+        /* Set the option bit, and incr/decr the invariants */
>+        opts |= 1 << (*ptr & 0x1F);
>+        len  -= olen;
>+        ptr  += olen;
>     }
> 
>     return opts;

Whitespace damage, I had to apply this manually, and in doing so,
reduced the size. You can test the patch from the "v4options" branch,
copied below.

>@@ -36,13 +54,20 @@
>     uint32_t opts = 0;
>     uint16_t len  = ip_hdrlen(skb) - sizeof(struct iphdr);
> 
>-    if (len > 0)
>+    if (len > 0) {
>         opts = ipv4options_rd((const void *)iph +
>                sizeof(struct iphdr), len);
>+        if(info->flags & XT_V4OPTS_ANY) {
>+            return  !!opts; /* !! to make bool value */
>+        }else {
> 
>     opts ^= info->invert;
>     opts &= info->map;
>-    return (info->flags & XT_V4OPTS_ANY) ? opts : opts == info->map;
>+            return (opts == info->map);
>+        }        
>+    }
>+    
>+    return 0;
> }

XT_V4OPTS_ANY is meant to match on {any of {the bits present in
info->map}}, not {any of {all possible IPv4 options}} according to
the documentation. The test for XT_V4OPTS_ANY is therefore supposed
to come after the ^ and & operations, and that is what the current
(/^-/) code does AFAICS, or did I miss something?





commit 75cd1d7d6a3747ec297f24f67f589acd8f5a9859
Author: Eivind Naess <eivnaes@xxxxxxxxx>
Date:   Thu Nov 3 09:28:46 2011 -0700

xt_ipv4options: fix an infinite loop
---
 doc/changelog.txt           |    1 +
 extensions/xt_ipv4options.c |   11 +++++++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/doc/changelog.txt b/doc/changelog.txt
index 3557175..81fcbdc 100644
--- a/doc/changelog.txt
+++ b/doc/changelog.txt
@@ -5,6 +5,7 @@ Fixes:
 - build: the code actually requires at least iptables 1.4.5 (would yield a
   compile error otherwise), make sure configure checks for it; update INSTALL
 - xt_ECHO: fix kernel warning about RTAX_HOPLIMIT being used
+- xt_ipv4options: fix an infinite loop
 Changes:
 - xt_ECHO: now calculates UDP checksum
 Enhancements:
diff --git a/extensions/xt_ipv4options.c b/extensions/xt_ipv4options.c
index 42481f7..5e9d34c 100644
--- a/extensions/xt_ipv4options.c
+++ b/extensions/xt_ipv4options.c
@@ -20,6 +20,17 @@ static uint32_t ipv4options_rd(const uint8_t *data, int len)
 	uint32_t opts = 0;
 
 	while (len >= 2) {
+		switch (data[0]) {
+		case IPOPT_END:
+			return opts;
+		case IPOPT_NOOP:
+			--len;
+			++data;
+			continue;
+		}
+
+		if (data[1] < 2 || data[1] > len)
+			return opts;
 		opts |= 1 << (data[0] & 0x1F);
 		len  -= data[1];
 		data += data[1];


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux