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 | |
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
-rw-r--r-- | dhcp-common.c | 65 | ||||
-rw-r--r-- | dhcp-common.h | 1 |
2 files changed, 31 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; 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 */ |