changeset 4499:94b77a3ab52f draft

DHCPv6: Fix a potential read overflow with D6_OPTION_PD_EXCLUDE dhcpcd only checks that the prefix length of the exclusion matches the prefix length of the ia and equals the length of the data in the option. This could potentially overrun the in6_addr structure. This is fixed by enforcing RFC 6603 section 4.2 option limits more clearly. Thanks to Maxime Villard <max@m00nbsd.net> for finding this.
author Roy Marples <roy@marples.name>
date Fri, 03 May 2019 14:44:06 +0100
parents 29e8b4603296
children be15f5b1966c
files src/dhcp6.c
diffstat 1 files changed, 22 insertions(+), 24 deletions(-) [+]
line wrap: on
line diff
--- a/src/dhcp6.c	Thu May 02 21:48:52 2019 +0100
+++ b/src/dhcp6.c	Fri May 03 14:44:06 2019 +0100
@@ -2166,40 +2166,38 @@
 			state->expire = a->prefix_vltime;
 		i++;
 
-		o = dhcp6_findoption(o, ol, D6_OPTION_PD_EXCLUDE, &ol);
 		a->prefix_exclude_len = 0;
 		memset(&a->prefix_exclude, 0, sizeof(a->prefix_exclude));
-#if 0
-		if (ex == NULL) {
-			struct dhcp6_option *w;
-			uint8_t *wp;
-
-			w = calloc(1, 128);
-			w->len = htons(2);
-			wp = D6_OPTION_DATA(w);
-			*wp++ = 64;
-			*wp++ = 0x78;
-			ex = w;
-		}
-#endif
+		o = dhcp6_findoption(o, ol, D6_OPTION_PD_EXCLUDE, &ol);
 		if (o == NULL)
 			continue;
-		if (ol < 2) {
-			logerrx("%s: truncated PD Exclude", ifp->name);
+
+		/* RFC 6603 4.2 says option length MUST be between 2 and 17.
+		 * This allows 1 octet for prefix length and 16 for the
+		 * subnet ID. */
+		if (ol < 2 || ol > 17) {
+			logerrx("%s: invalid PD Exclude option", ifp->name);
 			continue;
 		}
+
+		/* RFC 6603 4.2 says prefix length MUST be between the
+		 * length of the IAPREFIX prefix length + 1 and 128. */
+		if (*o < a->prefix_len + 1 || *o > 128) {
+			logerrx("%s: invalid PD Exclude length", ifp->name);
+			continue;
+		}
+
+		/* Check option length matches prefix length. */
+		if (((*o - a->prefix_len - 1) / NBBY) + 1 != ol) {
+			logerrx("%s: PD Exclude length mismatch", ifp->name);
+			continue;
+		}
+
 		a->prefix_exclude_len = *o++;
 		ol--;
-		if (((a->prefix_exclude_len - a->prefix_len - 1) / NBBY) + 1
-		    != ol)
-		{
-			logerrx("%s: PD Exclude length mismatch", ifp->name);
-			a->prefix_exclude_len = 0;
-			continue;
-		}
-		nb = a->prefix_len % NBBY;
 		memcpy(&a->prefix_exclude, &a->prefix,
 		    sizeof(a->prefix_exclude));
+		nb = a->prefix_len % NBBY;
 		if (nb)
 			ol--;
 		pw = a->prefix_exclude.s6_addr +