dhcpcd-discuss

Hi Chris

On 17/05/2019 08:39, Chris Lamb wrote:
> Hm, I don't quite follow. Would it be possible for you to quickly
> mockup what you mean? I would, naturally, be very happy to test.

Untested patch.
Basically I took the latest -6 function and made it work for 6.0.5.

Roy

diff --git a/dhcp.c b/dhcp.c
index 30c1ecd6..fb3734d9 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -75,6 +75,7 @@ static uint8_t *packet;
  * We ONLY use this when options are split, which for most purposes is
  * practically never. See RFC3396 for details. */
 static uint8_t *opt_buffer;
+static size_t opt_buffer_len;
 
 #define IPV4A		ADDRIPV4 | ARRAY
 #define IPV4R		ADDRIPV4 | REQUEST
@@ -295,65 +296,98 @@ free_option_buffer(void)
 static const uint8_t *
 get_option(const struct dhcp_message *dhcp, uint8_t opt, int *len, int *type)
 {
-	const uint8_t *p = dhcp->options;
-	const uint8_t *e = p + sizeof(dhcp->options);
-	uint8_t l, ol = 0;
-	uint8_t o = 0;
-	uint8_t overl = 0;
-	uint8_t *bp = NULL;
-	const uint8_t *op = NULL;
-	ssize_t bl = 0;
+	const uint8_t *p, *e;
+	uint8_t l, o, ol, overl, *bp;
+	const uint8_t *op;
+	size_t bl;
 
+	p = dhcp->options;
+	e = p + sizeof(dhcp->options);
+	ol = o = overl = 0;
+	bp = NULL;
+	op = NULL;
+	bl = 0;
 	while (p < e) {
 		o = *p++;
-		if (o == opt) {
-			if (op) {
-				if (!opt_buffer) {
-					opt_buffer = malloc(sizeof(*dhcp));
-					if (opt_buffer == NULL)
-						return NULL;
-#ifdef DEBUG_MEMORY
-					atexit(free_option_buffer);
-#endif
-				}
-				if (!bp)
-					bp = opt_buffer;
-				memcpy(bp, op, ol);
-				bp += ol;
-			}
-			ol = *p;
-			op = p + 1;
-			bl += ol;
-		}
 		switch (o) {
 		case DHO_PAD:
+			/* No length to read */
 			continue;
 		case DHO_END:
 			if (overl & 1) {
 				/* bit 1 set means parse boot file */
-				overl &= ~1;
+				overl = (uint8_t)(overl & ~1);
 				p = dhcp->bootfile;
 				e = p + sizeof(dhcp->bootfile);
 			} else if (overl & 2) {
 				/* bit 2 set means parse server name */
-				overl &= ~2;
+				overl = (uint8_t)(overl & ~2);
 				p = dhcp->servername;
 				e = p + sizeof(dhcp->servername);
 			} else
 				goto exit;
-			break;
-		case DHO_OPTIONSOVERLOADED:
-			/* Ensure we only get this option once */
-			if (!overl)
-				overl = p[1];
-			break;
+			/* No length to read */
+			continue;
+		}
+
+		/* Check we can read the length */
+		if (p == e) {
+			errno = EINVAL;
+		return NULL;
 		}
 		l = *p++;
+
+		/* Check we can read the option data, if present */
+		if (p + l > e) {
+			errno = EINVAL;
+			return NULL;
+		}
+
+		if (o == DHO_OPTIONSOVERLOADED) {
+			/* Ensure we only get this option once by setting
+			 * the last bit as well as the value.
+			 * This is valid because only the first two bits
+			 * actually mean anything in RFC2132 Section 9.3 */
+			if (l == 1 && !overl)
+				overl = 0x80 | p[0];
+		}
+
+		if (o == opt) {
+			if (op) {
+				/* We must concatonate the options. */
+				if (bl + l > opt_buffer_len) {
+					size_t pos;
+					uint8_t *nb;
+
+					if (bp)
+						pos = (size_t)
+						    (bp - opt_buffer);
+					else
+						pos = 0;
+					nb = realloc(opt_buffer, bl + l);
+					if (nb == NULL)
+						return NULL;
+#ifdef DEBUG_MEMORY
+					if (opt_buffer == NULL)
+						atexit(free_option_buffer);
+#endif
+					opt_buffer = nb;
+					opt_buffer_len = bl + l;
+					bp = opt_buffer + pos;
+				}
+				if (bp == NULL)
+					bp = opt_buffer;
+				memcpy(bp, op, ol);
+				bp += ol;
+			}
+			ol = l;
+			op = p;
+			bl += ol;
+		}
 		p += l;
 	}
 
 exit:
-
 	bl = validate_length(opt, bl, type);
 	if (bl == -1) {
 		errno = EINVAL;