Prevent crash of unprivileged engine process (pledged stdio).

The length field of a DNS label in the DNS search list option is an 8
bit unsigned value. parse_dnssl() treats the search list option as an
array of char, which are signed on most archs. When we read this value
into an int variable it gets sign extended, allowing it to bypass
sanity checks and eventually we pass it as the length to memcpy which
treats it as a huge unsigned value leading to a heap overflow.

An easy fix would be change the signature of parse_dnssl to
parse_dnssl(uint8_t* data, int datalen).

However, the DNS search list option is unused and the function fails
to check if the parsed value is a valid domain name. The function is
also getting in the way of future work so it's best to just delete it.

The problem was found and reported by qualys, thanks!

OK bluhm

3 files changed, 4 insertions, 137 deletions

diff --git a/sbin/slaacd/engine.c b/sbin/slaacd/engine.c
index 3270db8e5ae..164603c9e83 100644
--- a/sbin/slaacd/engine.c
+++ b/sbin/slaacd/engine.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: engine.c,v 1.76 2022/02/20 19:18:16 florian Exp $	*/
+/*	$OpenBSD: engine.c,v 1.77 2022/03/21 16:25:47 florian Exp $	*/
 
 /*
  * Copyright (c) 2017 Florian Obser <florian@openbsd.org>
@@ -156,11 +156,6 @@ struct radv_rdns {
 	struct in6_addr		rdns;
 };
 
-struct radv_dnssl {
-	LIST_ENTRY(radv_dnssl)	entries;
-	char			dnssl[SLAACD_MAX_DNSSL];
-};
-
 struct radv {
 	LIST_ENTRY(radv)		 entries;
 	struct sockaddr_in6		 from;
@@ -178,8 +173,6 @@ struct radv {
 	LIST_HEAD(, radv_prefix)	 prefixes;
 	uint32_t			 rdns_lifetime;
 	LIST_HEAD(, radv_rdns)		 rdns_servers;
-	uint32_t			 dnssl_lifetime;
-	LIST_HEAD(, radv_dnssl)		 dnssls;
 	uint32_t			 mtu;
 };
 
@@ -302,7 +295,6 @@ void			 gen_rdns_proposal(struct slaacd_iface *, struct
 void			 propose_rdns(struct rdns_proposal *);
 void			 free_rdns_proposal(struct rdns_proposal *);
 void			 compose_rdns_proposal(uint32_t, int);
-char			*parse_dnssl(char *, int);
 void			 update_iface_ra(struct slaacd_iface *, struct radv *);
 void			 update_iface_ra_dfr(struct slaacd_iface *,
     			     struct radv *);
@@ -765,14 +757,12 @@ send_interface_info(struct slaacd_iface *iface, pid_t pid)
 	struct ctl_engine_info_ra		 cei_ra;
 	struct ctl_engine_info_ra_prefix	 cei_ra_prefix;
 	struct ctl_engine_info_ra_rdns		 cei_ra_rdns;
-	struct ctl_engine_info_ra_dnssl		 cei_ra_dnssl;
 	struct ctl_engine_info_address_proposal	 cei_addr_proposal;
 	struct ctl_engine_info_dfr_proposal	 cei_dfr_proposal;
 	struct ctl_engine_info_rdns_proposal	 cei_rdns_proposal;
 	struct radv				*ra;
 	struct radv_prefix			*prefix;
 	struct radv_rdns			*rdns;
-	struct radv_dnssl			*dnssl;
 	struct address_proposal			*addr_proposal;
 	struct dfr_proposal			*dfr_proposal;
 	struct rdns_proposal			*rdns_proposal;
@@ -829,16 +819,6 @@ send_interface_info(struct slaacd_iface *iface, pid_t pid)
 			    IMSG_CTL_SHOW_INTERFACE_INFO_RA_RDNS, pid,
 			    &cei_ra_rdns, sizeof(cei_ra_rdns));
 		}
-
-		LIST_FOREACH(dnssl, &ra->dnssls, entries) {
-			memset(&cei_ra_dnssl, 0, sizeof(cei_ra_dnssl));
-			memcpy(&cei_ra_dnssl.dnssl, &dnssl->dnssl,
-			    sizeof(cei_ra_dnssl.dnssl));
-			cei_ra_dnssl.lifetime = ra->dnssl_lifetime;
-			engine_imsg_compose_frontend(
-			    IMSG_CTL_SHOW_INTERFACE_INFO_RA_DNSSL, pid,
-			    &cei_ra_dnssl, sizeof(cei_ra_dnssl));
-		}
 	}
 
 	if (!LIST_EMPTY(&iface->addr_proposals))
@@ -1036,7 +1016,6 @@ free_ra(struct radv *ra)
 {
 	struct radv_prefix	*prefix;
 	struct radv_rdns	*rdns;
-	struct radv_dnssl	*dnssl;
 
 	if (ra == NULL)
 		return;
@@ -1055,12 +1034,6 @@ free_ra(struct radv *ra)
 		free(rdns);
 	}
 
-	while (!LIST_EMPTY(&ra->dnssls)) {
-		dnssl = LIST_FIRST(&ra->dnssls);
-		LIST_REMOVE(dnssl, entries);
-		free(dnssl);
-	}
-
 	free(ra);
 }
 
@@ -1162,7 +1135,6 @@ parse_ra(struct slaacd_iface *iface, struct imsg_ra *ra)
 	struct radv		*radv;
 	struct radv_prefix	*prefix;
 	struct radv_rdns	*rdns;
-	struct radv_dnssl	*ra_dnssl;
 	ssize_t			 len = ra->len;
 	const char		*hbuf;
 	uint8_t			*p;
@@ -1200,7 +1172,6 @@ parse_ra(struct slaacd_iface *iface, struct imsg_ra *ra)
 
 	LIST_INIT(&radv->prefixes);
 	LIST_INIT(&radv->rdns_servers);
-	LIST_INIT(&radv->dnssls);
 
 	radv->min_lifetime = UINT32_MAX;
 
@@ -1251,11 +1222,9 @@ parse_ra(struct slaacd_iface *iface, struct imsg_ra *ra)
 		struct nd_opt_hdr *nd_opt_hdr = (struct nd_opt_hdr *)p;
 		struct nd_opt_prefix_info *prf;
 		struct nd_opt_rdnss *rdnss;
-		struct nd_opt_dnssl *dnssl;
 		struct nd_opt_mtu *mtu;
 		struct in6_addr *in6;
 		int i;
-		char *nssl;
 
 		len -= sizeof(struct nd_opt_hdr);
 		p += sizeof(struct nd_opt_hdr);
@@ -1323,37 +1292,6 @@ parse_ra(struct slaacd_iface *iface, struct imsg_ra *ra)
 				    entries);
 			}
 			break;
-		case ND_OPT_DNSSL:
-			if (nd_opt_hdr->nd_opt_len  < 2) {
-				log_warnx("invalid ND_OPT_DNSSL: len < 16");
-				goto err;
-			}
-
-			dnssl = (struct nd_opt_dnssl*) nd_opt_hdr;
-
-			if ((nssl = parse_dnssl(p + 6,
-			    (nd_opt_hdr->nd_opt_len - 1) * 8)) == NULL)
-				goto err; /* error logging in parse_dnssl */
-
-			if((ra_dnssl = calloc(1, sizeof(*ra_dnssl))) == NULL)
-				fatal("calloc");
-
-			radv->dnssl_lifetime = ntohl(
-			    dnssl->nd_opt_dnssl_lifetime);
-			if (radv->min_lifetime > radv->dnssl_lifetime)
-				radv->min_lifetime = radv->dnssl_lifetime;
-
-			if (strlcpy(ra_dnssl->dnssl, nssl,
-			    sizeof(ra_dnssl->dnssl)) >=
-			    sizeof(ra_dnssl->dnssl)) {
-				log_warnx("dnssl too long");
-				goto err;
-			}
-			free(nssl);
-
-			LIST_INSERT_HEAD(&radv->dnssls, ra_dnssl, entries);
-
-			break;
 		case ND_OPT_MTU:
 			if (nd_opt_hdr->nd_opt_len != 1) {
 				log_warnx("invalid ND_OPT_MTU: len != 1");
@@ -1369,6 +1307,7 @@ parse_ra(struct slaacd_iface *iface, struct imsg_ra *ra)
 			}
 
 			break;
+		case ND_OPT_DNSSL:
 		case ND_OPT_REDIRECTED_HEADER:
 		case ND_OPT_SOURCE_LINKADDR:
 		case ND_OPT_TARGET_LINKADDR:
@@ -1571,10 +1510,8 @@ debug_log_ra(struct imsg_ra *ra)
 		struct nd_opt_mtu *mtu;
 		struct nd_opt_prefix_info *prf;
 		struct nd_opt_rdnss *rdnss;
-		struct nd_opt_dnssl *dnssl;
 		struct in6_addr *in6;
 		int i;
-		char *nssl;
 
 		len -= sizeof(struct nd_opt_hdr);
 		p += sizeof(struct nd_opt_hdr);
@@ -1663,24 +1600,6 @@ debug_log_ra(struct imsg_ra *ra)
 				    ntopbuf, INET6_ADDRSTRLEN));
 			}
 			break;
-		case ND_OPT_DNSSL:
-			if (nd_opt_hdr->nd_opt_len  < 2) {
-				log_warnx("invalid ND_OPT_DNSSL: len < 16");
-				return;
-			}
-			dnssl = (struct nd_opt_dnssl*) nd_opt_hdr;
-			nssl = parse_dnssl(p + 6, (nd_opt_hdr->nd_opt_len - 1)
-			    * 8);
-
-			if (nssl == NULL)
-				return;
-
-			log_debug("\t\tND_OPT_DNSSL: lifetime: %u", ntohl(
-			    dnssl->nd_opt_dnssl_lifetime));
-			log_debug("\t\t\tsearch: %s", nssl);
-
-			free(nssl);
-			break;
 		default:
 			log_debug("\t\tUNKNOWN: %d", nd_opt_hdr->nd_opt_type);
 			break;
@@ -1692,48 +1611,6 @@ debug_log_ra(struct imsg_ra *ra)
 }
 #endif	/* SMALL */
 
-char*
-parse_dnssl(char* data, int datalen)
-{
-	int len, pos;
-	char *nssl, *nsslp;
-
-	if((nssl = calloc(1, datalen + 1)) == NULL) {
-		log_warn("malloc");
-		return NULL;
-	}
-	nsslp = nssl;
-
-	pos = 0;
-
-	do {
-		len = data[pos];
-		if (len > 63 || len + pos + 1 > datalen) {
-			free(nssl);
-			log_warnx("invalid label in DNSSL");
-			return NULL;
-		}
-		if (len == 0) {
-			if (pos < datalen && data[pos + 1] != 0)
-				*nsslp++ = ' '; /* seperator for next domain */
-			else
-				break;
-		} else {
-			if (pos != 0 && data[pos - 1] != 0) /* no . at front */
-				*nsslp++ = '.';
-			memcpy(nsslp, data + pos + 1, len);
-			nsslp += len;
-		}
-		pos += len + 1;
-	} while(pos < datalen);
-	if (len != 0) {
-		free(nssl);
-		log_warnx("invalid label in DNSSL");
-		return NULL;
-	}
-	return nssl;
-}
-
 void update_iface_ra(struct slaacd_iface *iface, struct radv *ra)
 {
 	struct radv		*old_ra;
diff --git a/sbin/slaacd/frontend.c b/sbin/slaacd/frontend.c
index b518f88939c..43105684daa 100644
--- a/sbin/slaacd/frontend.c
+++ b/sbin/slaacd/frontend.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: frontend.c,v 1.62 2022/01/04 06:17:46 florian Exp $	*/
+/*	$OpenBSD: frontend.c,v 1.63 2022/03/21 16:25:47 florian Exp $	*/
 
 /*
  * Copyright (c) 2017 Florian Obser <florian@openbsd.org>
@@ -419,7 +419,6 @@ frontend_dispatch_engine(int fd, short event, void *bula)
 		case IMSG_CTL_SHOW_INTERFACE_INFO_RA:
 		case IMSG_CTL_SHOW_INTERFACE_INFO_RA_PREFIX:
 		case IMSG_CTL_SHOW_INTERFACE_INFO_RA_RDNS:
-		case IMSG_CTL_SHOW_INTERFACE_INFO_RA_DNSSL:
 		case IMSG_CTL_SHOW_INTERFACE_INFO_ADDR_PROPOSALS:
 		case IMSG_CTL_SHOW_INTERFACE_INFO_ADDR_PROPOSAL:
 		case IMSG_CTL_SHOW_INTERFACE_INFO_DFR_PROPOSALS:
diff --git a/sbin/slaacd/slaacd.h b/sbin/slaacd/slaacd.h
index 034fb3a00e9..70349863574 100644
--- a/sbin/slaacd/slaacd.h
+++ b/sbin/slaacd/slaacd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: slaacd.h,v 1.35 2021/07/22 15:32:51 kn Exp $	*/
+/*	$OpenBSD: slaacd.h,v 1.36 2022/03/21 16:25:47 florian Exp $	*/
 
 /*
  * Copyright (c) 2017 Florian Obser <florian@openbsd.org>
@@ -25,9 +25,6 @@
 
 #define SLAACD_SOIIKEY_LEN	16
 
-/* MAXDNAME from arpa/namesr.h */
-#define SLAACD_MAX_DNSSL	1025
-
 #define	MAX_RDNS_COUNT		8 /* max nameserver in a RTM_PROPOSAL */
 
 #define	IMSG_DATA_SIZE(imsg)	((imsg).hdr.len - IMSG_HEADER_SIZE)
@@ -47,7 +44,6 @@ enum imsg_type {
 	IMSG_CTL_SHOW_INTERFACE_INFO_RA,
 	IMSG_CTL_SHOW_INTERFACE_INFO_RA_PREFIX,
 	IMSG_CTL_SHOW_INTERFACE_INFO_RA_RDNS,
-	IMSG_CTL_SHOW_INTERFACE_INFO_RA_DNSSL,
 	IMSG_CTL_SHOW_INTERFACE_INFO_ADDR_PROPOSALS,
 	IMSG_CTL_SHOW_INTERFACE_INFO_ADDR_PROPOSAL,
 	IMSG_CTL_SHOW_INTERFACE_INFO_DFR_PROPOSALS,
@@ -123,11 +119,6 @@ struct ctl_engine_info_ra_rdns {
 	struct in6_addr		rdns;
 };
 
-struct ctl_engine_info_ra_dnssl {
-	uint32_t		lifetime;
-	char			dnssl[SLAACD_MAX_DNSSL];
-};
-
 struct ctl_engine_info_address_proposal {
 	int64_t			 id;
 	char			 state[sizeof("PROPOSAL_NEARLY_EXPIRED")];