blob: 63cc313b8042734440624d5de71e8bdaba3a7b64 [file] [log] [blame]
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 */