| From ddbf02ae19016710a31fca4b2bc26f08a6fe191f Mon Sep 17 00:00:00 2001 |
| From: Samuel Tan <samueltan@google.com> |
| Date: Tue, 05 Jan 2016 11:15:54 -0800 |
| Subject: [PATCH] UPSTREAM: Fix heap-based overflow in dhcp_envoption1 |
| |
| 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. |
| |
| http://roy.marples.name/projects/dhcpcd/ci/76a1609352263bd9?sbs=0 |
| |
| BUG: 26402253 |
| |
| Reviewed-on: https://android-review.googlesource.com/#/c/194624/ |
| |
| --- |
| |
| diff --git a/dhcp-common.c b/dhcp-common.c |
| index a3764f1..588684e 100644 |
| --- a/dhcp-common.c |
| +++ b/dhcp-common.c |
| @@ -522,51 +522,43 @@ |
| return (ssize_t)bytes; |
| } |
| |
| -#define ADDRSZ 4 |
| #define ADDR6SZ 16 |
| static size_t |
| dhcp_optlen(const struct dhcp_opt *opt, size_t dl) |
| { |
| size_t sz; |
| |
| - if (dl == 0) |
| - return 0; |
| - |
| - if (opt->type == 0 || |
| - opt->type & (STRING | BINHEX | RFC3442 | RFC5969)) |
| - { |
| - if (opt->len) { |
| - if ((size_t)opt->len > dl) |
| - return 0; |
| - return (size_t)opt->len; |
| - } |
| - return dl; |
| - } |
| - |
| - if ((opt->type & (ADDRIPV4 | ARRAY)) == (ADDRIPV4 | ARRAY)) { |
| - if (dl < ADDRSZ) |
| - return 0; |
| - return dl - (dl % ADDRSZ); |
| - } |
| - |
| - if ((opt->type & (ADDRIPV6 | ARRAY)) == (ADDRIPV6 | ARRAY)) { |
| - if (dl < ADDR6SZ) |
| - return 0; |
| - return dl - (dl % ADDR6SZ); |
| - } |
| - |
| - if (opt->type & (UINT32 | ADDRIPV4)) |
| + 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) |
| + 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 dl; |
| - return (dl < sz ? 0 : sz); |
| + else if (opt->type & FLAG) |
| + return 0; |
| + else { |
| + /* All other types are variable length */ |
| + if (opt->len) { |
| + if ((size_t)opt->len > dl) { |
| + errno = ENODATA; |
| + return -1; |
| + } |
| + return (ssize_t)opt->len; |
| + } |
| + return (ssize_t)dl; |
| + } |
| + if (dl < sz) { |
| + errno = ENODATA; |
| + return -1; |
| + } |
| + |
| + /* 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; |
| } |
| |
| #ifdef INET6 |
| @@ -766,8 +758,11 @@ |
| 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->type, od, ol, ifname); |
| if (len < 0) |
| return 0; |
| diff --git a/dhcp-common.h b/dhcp-common.h |
| index a3d62fc..a87d32a 100644 |
| --- a/dhcp-common.h |
| +++ b/dhcp-common.h |
| @@ -66,6 +66,7 @@ |
| #define RAW (1 << 23) |
| #define ESCSTRING (1 << 24) |
| #define ESCFILE (1 << 25) |
| +#define BITFLAG (1 << 26) |
| |
| struct dhcp_opt { |
| uint32_t option; /* Also used for IANA Enterpise Number */ |