]> The Tcpdump Group git mirrors - tcpdump/commitdiff
When parsing information elements, check for the full length beforehand.
authorGuy Harris <[email protected]>
Sat, 19 Apr 2014 02:09:49 +0000 (19:09 -0700)
committerGuy Harris <[email protected]>
Sat, 19 Apr 2014 02:09:49 +0000 (19:09 -0700)
When parsing information elements, first check to make sure we have the
element ID and length, and fetch the length; then check to make sure we
have the entire element, including the information.  Remove those checks
from the handlers for individual elements.

This squelches a Coverity warning (when we check to make sure the length
remaining in the packet is enough for the element ID; the element ID is
one byte, and the loop continues as long as the length is non-zero, so
that's always true in the loop), and simplifies some other code.

Also check for the right length for fixed-length elements while we're at
it.

print-802_11.c

index c94cc0dda955980d1107ab76af12df7b5d100eb2..3628f3dc08ec7b1e662cbbe85bafa31ec71d9c8f 100644 (file)
@@ -1315,16 +1315,21 @@ parse_elements(netdissect_options *ndo,
        pbody->tim_present = 0;
 
        while (length != 0) {
-               if (!ND_TTEST2(*(p + offset), 1))
+               /* Make sure we at least have the element ID and length. */
+               if (!ND_TTEST2(*(p + offset), 2))
                        return 0;
-               if (length < 1)
+               if (length < 2)
                        return 0;
+               elementlen = *(p + offset + 1);
+
+               /* Make sure we have the entire element. */
+               if (!ND_TTEST2(*(p + offset + 2), elementlen))
+                       return 0;
+               if (length < elementlen + 2)
+                       return 0;
+
                switch (*(p + offset)) {
                case E_SSID:
-                       if (!ND_TTEST2(*(p + offset), 2))
-                               return 0;
-                       if (length < 2)
-                               return 0;
                        memcpy(&ssid, p + offset, 2);
                        offset += 2;
                        length -= 2;
@@ -1353,10 +1358,6 @@ parse_elements(netdissect_options *ndo,
                        }
                        break;
                case E_CHALLENGE:
-                       if (!ND_TTEST2(*(p + offset), 2))
-                               return 0;
-                       if (length < 2)
-                               return 0;
                        memcpy(&challenge, p + offset, 2);
                        offset += 2;
                        length -= 2;
@@ -1387,10 +1388,6 @@ parse_elements(netdissect_options *ndo,
                        }
                        break;
                case E_RATES:
-                       if (!ND_TTEST2(*(p + offset), 2))
-                               return 0;
-                       if (length < 2)
-                               return 0;
                        memcpy(&rates, p + offset, 2);
                        offset += 2;
                        length -= 2;
@@ -1427,13 +1424,17 @@ parse_elements(netdissect_options *ndo,
                        }
                        break;
                case E_DS:
-                       if (!ND_TTEST2(*(p + offset), 3))
-                               return 0;
-                       if (length < 3)
-                               return 0;
-                       memcpy(&ds, p + offset, 3);
-                       offset += 3;
-                       length -= 3;
+                       memcpy(&ds, p + offset, 2);
+                       offset += 2;
+                       length -= 2;
+                       if (ds.length != 1) {
+                               offset += ds.length;
+                               length -= ds.length;
+                               break;
+                       }
+                       ds.channel = *(p + offset);
+                       offset += 1;
+                       length -= 1;
                        /*
                         * Present and not truncated.
                         *
@@ -1447,13 +1448,17 @@ parse_elements(netdissect_options *ndo,
                        }
                        break;
                case E_CF:
-                       if (!ND_TTEST2(*(p + offset), 8))
-                               return 0;
-                       if (length < 8)
-                               return 0;
-                       memcpy(&cf, p + offset, 8);
-                       offset += 8;
-                       length -= 8;
+                       memcpy(&cf, p + offset, 2);
+                       offset += 2;
+                       length -= 2;
+                       if (cf.length != 6) {
+                               offset += cf.length;
+                               length -= cf.length;
+                               break;
+                       }
+                       memcpy(&cf.count, p + offset, 6);
+                       offset += 6;
+                       length -= 6;
                        /*
                         * Present and not truncated.
                         *
@@ -1467,29 +1472,20 @@ parse_elements(netdissect_options *ndo,
                        }
                        break;
                case E_TIM:
-                       if (!ND_TTEST2(*(p + offset), 2))
-                               return 0;
-                       if (length < 2)
-                               return 0;
                        memcpy(&tim, p + offset, 2);
                        offset += 2;
                        length -= 2;
-                       if (!ND_TTEST2(*(p + offset), 3))
-                               return 0;
-                       if (length < 3)
+                       if (tim.length <= 3) {
+                               offset += tim.length;
+                               length -= tim.length;
+                               break;
+                       }
+                       if (tim.length - 3 > (int)sizeof tim.bitmap)
                                return 0;
                        memcpy(&tim.count, p + offset, 3);
                        offset += 3;
                        length -= 3;
 
-                       if (tim.length <= 3)
-                               break;
-                       if (tim.length - 3 > (int)sizeof tim.bitmap)
-                               return 0;
-                       if (!ND_TTEST2(*(p + offset), tim.length - 3))
-                               return 0;
-                       if (length < (u_int)(tim.length - 3))
-                               return 0;
                        memcpy(tim.bitmap, p + (tim.length - 3),
                            (tim.length - 3));
                        offset += tim.length - 3;
@@ -1511,17 +1507,8 @@ parse_elements(netdissect_options *ndo,
                        ND_PRINT((ndo, "(1) unhandled element_id (%d)  ",
                            *(p + offset)));
 #endif
-                       if (!ND_TTEST2(*(p + offset), 2))
-                               return 0;
-                       if (length < 2)
-                               return 0;
-                       elementlen = *(p + offset + 1);
-                       if (!ND_TTEST2(*(p + offset + 2), elementlen))
-                               return 0;
-                       if (length < elementlen + 2)
-                               return 0;
-                       offset += elementlen + 2;
-                       length -= elementlen + 2;
+                       offset += 2 + elementlen;
+                       length -= 2 + elementlen;
                        break;
                }
        }