diff options
author | Samuel Tan <samueltan@google.com> | 2016-01-05 11:15:54 -0800 |
---|---|---|
committer | Samuel Tan <samueltan@google.com> | 2016-01-05 11:43:44 -0800 |
commit | ddbf02ae19016710a31fca4b2bc26f08a6fe191f (patch) | |
tree | 0ad17842002254eac80dae3b5aeb7ffd07d8e50b /dhcp-common.c | |
parent | e8e1f37493e07922bcb8be22ab9e2fe76e861b05 (diff) | |
download | dhcpcd-6.8.2-ddbf02ae19016710a31fca4b2bc26f08a6fe191f.tar.gz |
[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
Change-Id: Ic3c73277bb1cc09be08b3fbceecb97e2effd05ff
Diffstat (limited to 'dhcp-common.c')
-rw-r--r-- | dhcp-common.c | 65 |
1 files changed, 30 insertions, 35 deletions
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 @@ 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 size_t dhcp_optlen(const struct dhcp_opt *opt, size_t dl) { size_t sz; - if (dl == 0) + 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; - - if (opt->type == 0 || - opt->type & (STRING | BINHEX | RFC3442 | RFC5969)) - { + else { + /* All other types are variable length */ if (opt->len) { - if ((size_t)opt->len > dl) - return 0; - return (size_t)opt->len; + if ((size_t)opt->len > dl) { + errno = ENODATA; + return -1; + } + return (ssize_t)opt->len; } - return dl; - } - - if ((opt->type & (ADDRIPV4 | ARRAY)) == (ADDRIPV4 | ARRAY)) { - if (dl < ADDRSZ) - return 0; - return dl - (dl % ADDRSZ); + return (ssize_t)dl; } - - if ((opt->type & (ADDRIPV6 | ARRAY)) == (ADDRIPV6 | ARRAY)) { - if (dl < ADDR6SZ) - return 0; - return 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) - 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); + /* 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 @@ dhcp_envoption1(struct dhcpcd_ctx *ctx, 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->type, od, ol, ifname); if (len < 0) return 0; |