udhcpc: fix OPTION_6RD parsing (could overflow its malloced buffer)
authorDenys Vlasenko <vda.linux@googlemail.com>
Fri, 26 Feb 2016 14:54:56 +0000 (15:54 +0100)
committerDenys Vlasenko <vda.linux@googlemail.com>
Fri, 26 Feb 2016 14:54:56 +0000 (15:54 +0100)
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
networking/udhcp/common.c
networking/udhcp/dhcpc.c

index bc41c8d4d8e2a2fe321838f3c2ae0531c7aa7caa..680852ce4d7cbc0a69104e86574a0e0b3580221b 100644 (file)
@@ -142,7 +142,7 @@ const char dhcp_option_strings[] ALIGN1 =
  * udhcp_str2optset: to determine how many bytes to allocate.
  * xmalloc_optname_optval: to estimate string length
  * from binary option length: (option[LEN] / dhcp_option_lengths[opt_type])
- * is the number of elements, multiply in by one element's string width
+ * is the number of elements, multiply it by one element's string width
  * (len_of_option_as_string[opt_type]) and you know how wide string you need.
  */
 const uint8_t dhcp_option_lengths[] ALIGN1 = {
@@ -162,7 +162,18 @@ const uint8_t dhcp_option_lengths[] ALIGN1 = {
        [OPTION_S32] =     4,
        /* Just like OPTION_STRING, we use minimum length here */
        [OPTION_STATIC_ROUTES] = 5,
-       [OPTION_6RD] =    22,  /* ignored by udhcp_str2optset */
+       [OPTION_6RD] =    12,  /* ignored by udhcp_str2optset */
+       /* The above value was chosen as follows:
+        * len_of_option_as_string[] for this option is >60: it's a string of the form
+        * "32 128 ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff 255.255.255.255 ".
+        * Each additional ipv4 address takes 4 bytes in binary option and appends
+        * another "255.255.255.255 " 16-byte string. We can set [OPTION_6RD] = 4
+        * but this severely overestimates string length: instead of 16 bytes,
+        * it adds >60 for every 4 bytes in binary option.
+        * We cheat and declare here that option is in units of 12 bytes.
+        * This adds more than 60 bytes for every three ipv4 addresses - more than enough.
+        * (Even 16 instead of 12 should work, but let's be paranoid).
+        */
 };
 
 
index 48097bc2431b7850f374966580ab5d151bd40b52..2fe84e1cae8e5f50e1fc5d6d51f263280389fcdd 100644 (file)
@@ -113,7 +113,7 @@ static const uint8_t len_of_option_as_string[] = {
        [OPTION_IP              ] = sizeof("255.255.255.255 "),
        [OPTION_IP_PAIR         ] = sizeof("255.255.255.255 ") * 2,
        [OPTION_STATIC_ROUTES   ] = sizeof("255.255.255.255/32 255.255.255.255 "),
-       [OPTION_6RD             ] = sizeof("32 128 ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff 255.255.255.255 "),
+       [OPTION_6RD             ] = sizeof("132 128 ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff 255.255.255.255 "),
        [OPTION_STRING          ] = 1,
        [OPTION_STRING_HOST     ] = 1,
 #if ENABLE_FEATURE_UDHCP_RFC3397
@@ -222,7 +222,7 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_
        type = optflag->flags & OPTION_TYPE_MASK;
        optlen = dhcp_option_lengths[type];
        upper_length = len_of_option_as_string[type]
-               * ((unsigned)(len + optlen - 1) / (unsigned)optlen);
+               * ((unsigned)(len + optlen) / (unsigned)optlen);
 
        dest = ret = xmalloc(upper_length + strlen(opt_name) + 2);
        dest += sprintf(ret, "%s=", opt_name);