352f79acb
Denys Vlasenko
2016-02-26
Subject
udhcpc: fix OPTION_6RD parsing (could overflow its malloced buffer)
Binary
No binary (build failed)
Architecture & Compiler Flags
—
Build Log
(log yok)
Commit Diff GitHub →
networking/udhcp/common.c+13 -2
| @@ -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). |
| + | */ |
| }; | |
networking/udhcp/dhcpc.c+2 -2
| @@ -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); | |