CVE-2021-43523: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: In uClibc and uClibc-ng before 1.0.39, incorrect handling of special characters in domain names returned by DNS servers via gethostbyname, getaddrinfo, gethostbyaddr, and getnameinfo can lead to output of wrong hostnames (leading to domain hijacking) or injection into applications (leading to remote code execution, XSS, applications crashes, etc.). In other words, a validation step, which is expected in any stub resolver, does not occur.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 9.6
Published: 2021-11-10T15:15Z
Repository: https://github.com/wbx-github/uclibc-ng
Language: C
Files changed: libc/inet/resolv.c
Commit date: 2021-08-09 03:37:31 +0200
Commit message: libc/inet/resolv.c: add __hnbad to check DNS entries for validity…

… using the same rules glibc does

also call __hnbad in some places to check answers

Diff: diff --git a/libc/inet/resolv.c b/libc/inet/resolv.c
index ced3b6542..8bbd7c7cd 100644
--- a/libc/inet/resolv.c
+++ b/libc/inet/resolv.c
@@ -407,10 +407,11 @@ extern int __encode_question(const struct resolv_question *q,
 extern int __encode_answer(struct resolv_answer *a,
 		unsigned char *dest,
 		int maxlen) attribute_hidden;
 extern void __open_nameservers(void) attribute_hidden;
 extern void __close_nameservers(void) attribute_hidden;
+extern int __hnbad(const char *dotted) attribute_hidden;
 
 #define __encode_dotted(dotted,dest,maxlen) \
 	dn_comp((dotted), (dest), (maxlen), NULL, NULL)
 #define __decode_dotted(packet,offset,packet_len,dest,dest_len) \
 	dn_expand((packet), (packet) + (packet_len), (packet) + (offset), \
@@ -1091,10 +1092,11 @@ int __dns_lookup(const char *name,
 	int local_id = local_id; /* for compiler */
 	int sdomains = 0;
 	bool ends_with_dot;
 	bool contains_dot;
 	sockaddr46_t sa;
+	int num_answers;
 
 	fd = -1;
 	lookup = NULL;
 	name_len = strlen(name);
 	if ((unsigned)name_len >= MAXDNAME - MAXLEN_searchdomain - 2)
@@ -1333,10 +1335,11 @@ int __dns_lookup(const char *name,
 		if (h.ancount <= 0) {
 			h_errno = NO_DATA; /* [is this correct code to check for?] */
 			goto fail1;
 		}
 		pos = HFIXEDSZ;
+		/*XXX TODO: check that question matches query (and qdcount==1?) */
 		for (j = 0; j < h.qdcount; j++) {
 			DPRINTF("Skipping question %d at %d\n", j, pos);
 			i = __length_question(packet + pos, packet_len - pos);
 			if (i < 0) {
 				DPRINTF("Packet'question section "
@@ -1347,23 +1350,27 @@ int __dns_lookup(const char *name,
 			DPRINTF("Length of question %d is %d\n", j, i);
 		}
 		DPRINTF("Decoding answer at pos %d\n", pos);
 
 		first_answer = 1;
+		num_answers = 0;
 		a->dotted = NULL;
 		for (j = 0; j < h.ancount; j++) {
 			i = __decode_answer(packet, pos, packet_len, &ma);
 			if (i < 0) {
 				DPRINTF("failed decode %d\n", i);
 				/* If the message was truncated but we have
 				 * decoded some answers, pretend it's OK */
-				if (j && h.tc)
+				if (num_answers && h.tc)
 					break;
 				goto try_next_server;
 			}
 			pos += i;
 
+			if (__hnbad(ma.dotted))
+				break;
+			++num_answers;
 			if (first_answer) {
 				ma.buf = a->buf;
 				ma.buflen = a->buflen;
 				ma.add_count = a->add_count;
 				free(a->dotted);
@@ -1389,10 +1396,14 @@ int __dns_lookup(const char *name,
 				}
 				memcpy(a->buf + (a->add_count * ma.rdlength), ma.rdata, ma.rdlength);
 				++a->add_count;
 			}
 		}
+		if (!num_answers) {
+			h_errno = NO_RECOVERY;
+			goto fail1;
+		}
 
 		/* Success! */
 		DPRINTF("Answer name = |%s|\n", a->dotted);
 		DPRINTF("Answer type = |%d|\n", a->atype);
 		if (fd != -1)
@@ -2355,19 +2366,23 @@ int gethostbyaddr_r(const void *addr, socklen_t addrlen,
 			return -1;
 		}
 		/* Decode CNAME into buf, feed it to __dns_lookup() again */
 		i = __decode_dotted(packet, a.rdoffset, packet_len, buf, buflen);
 		free(packet);
-		if (i < 0) {
+		if (i < 0 || __hnbad(buf)) {
 			*h_errnop = NO_RECOVERY;
 			return -1;
 		}
 	}
 
 	if (a.atype == T_PTR) {	/* ADDRESS */
 		i = __decode_dotted(packet, a.rdoffset, packet_len, buf, buflen);
 		free(packet);
+		if (__hnbad(buf)) {
+			*h_errnop = NO_RECOVERY;
+			return -1;
+		}
 		result_buf->h_name = buf;
 		result_buf->h_addrtype = type;
 		result_buf->h_length = addrlen;
 		result_buf->h_addr_list = (char **) addr_list;
 		result_buf->h_aliases = alias;
@@ -2928,10 +2943,55 @@ int ns_name_pton(const char *src, u_char *dst, size_t dstsiz)
 	errno = EMSGSIZE;
 	return -1;
 }
 libc_hidden_def(ns_name_pton)
 
+/*
+ * __hnbad(dotted)
+ *	Check whether a name is valid enough for DNS. The rules, as
+ *	laid down by glibc, are:
+ *	- printable input string
+ *	- converts to label notation
+ *	- each label only contains [0-9a-zA-Z_-], up to 63 octets
+ *	- first label doesn’t begin with ‘-’
+ *	This both is weaker than Unix hostnames (e.g. it allows
+ *	underscores and leading/trailing hyphen-minus) and stronger
+ *	than general (e.g. a leading “*.” is valid sometimes), take care.
+ * return:
+ *	0 if the name is ok
+ */
+int __hnbad(const char *dotted)
+{
+	unsigned char c, n, *cp;
+	unsigned char buf[NS_MAXCDNAME];
+
+	cp = (unsigned char *)dotted;
+	while ((c = *cp++))
+		if (c < 0x21 || c > 0x7E)
+			return (1);
+	if (ns_name_pton(dotted, buf, sizeof(buf)) < 0)
+		return (2);
+	if (buf[0] > 0 && buf[1] == '-')
+		return (3);
+	cp = buf;
+	while ((n = *cp++)) {
+		if (n > 63)
+			return (4);
+		while (n--) {
+			c = *cp++;
+			if (c < '-' ||
+			    (c > '-' && c < '0') ||
+			    (c > '9' && c < 'A') ||
+			    (c > 'Z' && c < '_') ||
+			    (c > '_' && c < 'a') ||
+			    c > 'z')
+				return (5);
+		}
+	}
+	return (0);
+}
+
 /*
  * ns_name_unpack(msg, eom, src, dst, dstsiz)
  *      Unpack a domain name from a message, source may be compressed.
  * return:
  *      -1 if it fails, or consumed octets if it succeeds.

Vulnerable code: 				if (j && h.tc)
		if (i < 0) {
Fixed code: extern int __hnbad(const char *dotted) attribute_hidden;
	int num_answers;
		/*XXX TODO: check that question matches query (and qdcount==1?) */
		num_answers = 0;
				if (num_answers && h.tc)
			if (__hnbad(ma.dotted))
				break;
			++num_answers;
		if (!num_answers) {
			h_errno = NO_RECOVERY;
			goto fail1;
		}
		if (i < 0 || __hnbad(buf)) {
		if (__hnbad(buf)) {
			*h_errnop = NO_RECOVERY;
			return -1;
		}
/*
 * __hnbad(dotted)
 *	Check whether a name is valid enough for DNS. The rules, as
 *	laid down by glibc, are:
 *	- printable input string
 *	- converts to label notation
 *	- each label only contains [0-9a-zA-Z_-], up to 63 octets
 *	- first label doesn’t begin with ‘-’
 *	This both is weaker than Unix hostnames (e.g. it allows
 *	underscores and leading/trailing hyphen-minus) and stronger
 *	than general (e.g. a leading “*.” is valid sometimes), take care.
 * return:
 *	0 if the name is ok
 */
int __hnbad(const char *dotted)
{
	unsigned char c, n, *cp;
	unsigned char buf[NS_MAXCDNAME];

	cp = (unsigned char *)dotted;
	while ((c = *cp++))
		if (c < 0x21 || c > 0x7E)
			return (1);
	if (ns_name_pton(dotted, buf, sizeof(buf)) < 0)
		return (2);
	if (buf[0] > 0 && buf[1] == '-')
		return (3);
	cp = buf;
	while ((n = *cp++)) {
		if (n > 63)
			return (4);
		while (n--) {
			c = *cp++;
			if (c < '-' ||
			    (c > '-' && c < '0') ||
			    (c > '9' && c < 'A') ||
			    (c > 'Z' && c < '_') ||
			    (c > '_' && c < 'a') ||
			    c > 'z')
				return (5);
		}
	}
	return (0);
}