shim buffer overflow on ipv6 option parsing

netboot.c

@@ -108,29 +108,34 @@ BOOLEAN findNetboot(EFI_HANDLE device)
 
 static CHAR8 *get_v6_bootfile_url(EFI_PXE_BASE_CODE_DHCPV6_PACKET *pkt)
 {
-	void *optr;
+	void *optr = NULL, *end = NULL;
-	EFI_DHCP6_PACKET_OPTION *option;
+	EFI_DHCP6_PACKET_OPTION *option = NULL;
-	CHAR8 *url;
+	CHAR8 *url = NULL;
-	UINT32 urllen;
+	UINT32 urllen = 0;
 
 	optr = pkt->DhcpOptions;
+	end = optr + sizeof(pkt->DhcpOptions);
 
-	for(;;) {
+	for (;;) {
 		option = (EFI_DHCP6_PACKET_OPTION *)optr;
 
 		if (ntohs(option->OpCode) == 0)
-			return NULL;
+			break;
 
 		if (ntohs(option->OpCode) == 59) {
 			/* This is the bootfile url option */
 			urllen = ntohs(option->Length);
-			url = AllocateZeroPool(urllen+1);
+			if ((void *)(option->Data + urllen) > end)
+				break;
+			url = AllocateZeroPool(urllen + 1);
 			if (!url)
-				return NULL;
+				break;
 			memcpy(url, option->Data, urllen);
 			return url;
 		}
 		optr += 4 + ntohs(option->Length);
+		if (optr + sizeof(EFI_DHCP6_PACKET_OPTION) > end)
+			break;
 	}
 
 	return NULL;
@@ -156,45 +161,60 @@ static CHAR16 str2ns(CHAR8 *str)
 
 static CHAR8 *str2ip6(CHAR8 *str)
 {
-        UINT8 i, j, p;
+	UINT8 i = 0, j = 0, p = 0;
-	size_t len;
+	size_t len = 0, dotcount = 0;
-        CHAR8 *a, *b, t;
+	enum { MAX_IP6_DOTS = 7 };
-        static UINT16 ip[8];
+	CHAR8 *a = NULL, *b = NULL, t = 0;
+	static UINT16 ip[8];
 
-        for(i=0; i < 8; i++) {
+	memset(ip, 0, sizeof(ip));
-                ip[i] = 0;
+
-        }
+	/* Count amount of ':' to prevent overflows.
-        len = strlen(str);
+	 * max. count = 7. Returns an invalid ip6 that
-        a = b = str;
+	 * can be checked against
-        for(i=p=0; i < len; i++, b++) {
+	 */
-                if (*b != ':')
+	for (a = str; *a != 0; ++a) {
-                        continue;
+		if (*a == ':')
-                *b = '\0';
+			++dotcount;
-                ip[p++] = str2ns(a);
+	}
-                *b = ':';
+	if (dotcount > MAX_IP6_DOTS)
-                a = b + 1;
+		return (CHAR8 *)ip;
-                if ( *(b+1) == ':' )
+
-                        break;
+	len = strlen(str);
-        }
+	a = b = str;
-        a = b = (str + len);
+	for (i = p = 0; i < len; i++, b++) {
-        for(j=len, p=7; j > i; j--, a--) {
+		if (*b != ':')
-                if (*a != ':')
+			continue;
-                        continue;
+		*b = '\0';
-                t = *b;
+		ip[p++] = str2ns(a);
-                *b = '\0';
+		*b = ':';
-                ip[p--] = str2ns(a+1);
+		a = b + 1;
-                *b = t;
+		if (b[1] == ':' )
-                b = a;
+			break;
-        }
+	}
-        return (CHAR8 *)ip;
+	a = b = (str + len);
+	for (j = len, p = 7; j > i; j--, a--) {
+		if (*a != ':')
+			continue;
+		t = *b;
+		*b = '\0';
+		ip[p--] = str2ns(a+1);
+		*b = t;
+		b = a;
+	}
+	return (CHAR8 *)ip;
 }
 
 static BOOLEAN extract_tftp_info(CHAR8 *url)
 {
 	CHAR8 *start, *end;
 	CHAR8 ip6str[40];
+	CHAR8 ip6inv[16];
 	CHAR8 *template = (CHAR8 *)translate_slashes(DEFAULT_LOADER_CHAR);
 
+	// to check against str2ip6() errors
+	memset(ip6inv, 0, sizeof(ip6inv));
+
 	if (strncmp((UINT8 *)url, (UINT8 *)"tftp://", 7)) {
 		Print(L"URLS MUST START WITH tftp://\n");
 		return FALSE;
@@ -209,7 +229,7 @@ static BOOLEAN extract_tftp_info(CHAR8 *url)
 	end = start;
 	while ((*end != '\0') && (*end != ']')) {
 		end++;
-		if (end - start > 39) {
+		if (end - start >= (int)sizeof(ip6str)) {
 			Print(L"TFTP URL includes malformed IPv6 address\n");
 			return FALSE;
 		}
@@ -218,10 +238,12 @@ static BOOLEAN extract_tftp_info(CHAR8 *url)
 		Print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n");
 		return FALSE;
 	}
-	memset(ip6str, 0, 40);
+	memset(ip6str, 0, sizeof(ip6str));
 	memcpy(ip6str, start, end - start);
 	end++;
 	memcpy(&tftp_addr.v6, str2ip6(ip6str), 16);
+	if (memcmp(&tftp_addr.v6, ip6inv, sizeof(ip6inv)) == 0)
+		return FALSE;
 	full_path = AllocateZeroPool(strlen(end)+strlen(template)+1);
 	if (!full_path)
 		return FALSE;