dhcp_optlen now returns the length of the data we can sanely work on

given the option definition and data length.
Call dhcp_optlen in dhcp_envoption1 to take into ensure these bounds
are not overstepped.

Fixes an issue reported by Nico Golde where extra undersized data was
present in the option.
An example of this would be an array of uint16's with a trailing byte.

Fixes CVE-2016-1503

diff --git a/dhcp-common.c b/dhcp-common.c
index 3e064c35e22776de6d18f38780b05168d48d6017..d8b04a7bfdf762b70b86ff66c8a3f6cbbb895884 100644 (file)
--- a/dhcp-common.c
+++ b/dhcp-common.c
@@ -579,48 +579,43 @@ print_string(char *dst, size_t len, int type, const uint8_t *data, size_t dl)
        return (ssize_t)bytes;
 }
 
-#define ADDRSZ         4
 #define ADDR6SZ                16
 static ssize_t
 dhcp_optlen(const struct dhcp_opt *opt, size_t dl)
 {
        size_t sz;
 
-       if (opt->type == 0 ||
-           opt->type & (STRING | BINHEX | RFC3442))
-       {
+       if (opt->type & ADDRIPV6)
+               sz = ADDR6SZ;
+       else if (opt->type & (UINT32 | ADDRIPV4))
+               sz = sizeof(uint32_t);
+       else if (opt->type & UINT16)
+               sz = sizeof(uint16_t);
+       else if (opt->type & (UINT8 | BITFLAG))
+               sz = sizeof(uint8_t);
+       else if (opt->type & FLAG)
+               return 0;
+       else {
+               /* All other types are variable length */
                if (opt->len) {
-                       if ((size_t)opt->len > dl)
+                       if ((size_t)opt->len > dl) {
+                               errno = ENODATA;
                                return -1;
+                       }
                        return (ssize_t)opt->len;
                }
                return (ssize_t)dl;
        }
-
-       if ((opt->type & (ADDRIPV4 | ARRAY)) == (ADDRIPV4 | ARRAY)) {
-               if (dl < ADDRSZ)
-                       return -1;
-               return (ssize_t)(dl - (dl % ADDRSZ));
-       }
-
-       if ((opt->type & (ADDRIPV6 | ARRAY)) == (ADDRIPV6 | ARRAY)) {
-               if (dl < ADDR6SZ)
-                       return -1;
-               return (ssize_t)(dl - (dl % ADDR6SZ));
+       if (dl < sz) {
+               errno = ENODATA;
+               return -1;
        }
 
-       if (opt->type & (UINT32 | ADDRIPV4))
-               sz = sizeof(uint32_t);
-       else if (opt->type & UINT16)
-               sz = sizeof(uint16_t);
-       else if (opt->type & (UINT8 | BITFLAG))
-               sz = sizeof(uint8_t);
-       else if (opt->type & ADDRIPV6)
-               sz = ADDR6SZ;
-       else
-               /* If we don't know the size, assume it's valid */
-               return (ssize_t)dl;
-       return dl < sz ? -1 : (ssize_t)sz;
+       /* Trim any extra data.
+        * Maybe we need a settng to reject DHCP options with extra data? */
+       if (opt->type & ARRAY)
+               return (ssize_t)(dl - (dl % sz));
+       return (ssize_t)sz;
 }
 
 static ssize_t
@@ -843,8 +838,11 @@ dhcp_envoption1(char **env, const char *prefix,
        size_t e;
        char *v, *val;
 
-       if (opt->len && opt->len < ol)
-               ol = opt->len;
+       /* Ensure a valid length */
+       ol = (size_t)dhcp_optlen(opt, ol);
+       if ((ssize_t)ol == -1)
+               return 0;
+
        len = print_option(NULL, 0, opt, od, ol, ifname);
        if (len < 0)
                return 0;