From: bluhm <bluhm@openbsd.org>
Date: Sat, 6 Apr 2024 14:23:27 +0000 (+0000)
Subject: IP multicast sysctl mrtmfc must not write outside of allocation.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=85fbf21ae5ae715c1219116d9e0049b963b7b3c6;p=openbsd

IP multicast sysctl mrtmfc must not write outside of allocation.

Reading sysctl mrt_sysctl_mfc() allocates memory to be copied back
to user.  Chunks of struct mfcinfo are copied from routing table
to linear heap memory.  If the allocated memory was not a multiple
the struct size, a struct mfcinfo could be copied to a partially
unallocated destination.  Check that the end of the struct is within
the allocation.

From Alfredo Ortega;  OK claudio@
---

diff --git a/sys/netinet/ip_mroute.c b/sys/netinet/ip_mroute.c
index 3e9b00ec265..1913b1767a1 100644
--- a/sys/netinet/ip_mroute.c
+++ b/sys/netinet/ip_mroute.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_mroute.c,v 1.141 2024/02/11 18:14:26 mvs Exp $	*/
+/*	$OpenBSD: ip_mroute.c,v 1.142 2024/04/06 14:23:27 bluhm Exp $	*/
 /*	$NetBSD: ip_mroute.c,v 1.85 2004/04/26 01:31:57 matt Exp $	*/
 
 /*
@@ -430,8 +430,9 @@ mrt_rtwalk_mfcsysctl(struct rtentry *rt, void *arg, unsigned int rtableid)
 	}
 
 	for (minfo = msa->msa_minfos;
-	     (uint8_t *)minfo < ((uint8_t *)msa->msa_minfos + msa->msa_len);
-	     minfo++) {
+	    (uint8_t *)(minfo + 1) <=
+	    (uint8_t *)msa->msa_minfos + msa->msa_len;
+	    minfo++) {
 		/* Find a new entry or update old entry. */
 		if (minfo->mfc_origin.s_addr !=
 		    satosin(rt->rt_gateway)->sin_addr.s_addr ||
@@ -471,13 +472,11 @@ mrt_sysctl_mfc(void *oldp, size_t *oldlenp)
 	if (oldp != NULL && *oldlenp > MAXPHYS)
 		return (EINVAL);
 
-	if (oldp != NULL)
+	memset(&msa, 0, sizeof(msa));
+	if (oldp != NULL && *oldlenp > 0) {
 		msa.msa_minfos = malloc(*oldlenp, M_TEMP, M_WAITOK | M_ZERO);
-	else
-		msa.msa_minfos = NULL;
-
-	msa.msa_len = *oldlenp;
-	msa.msa_needed = 0;
+		msa.msa_len = *oldlenp;
+	}
 
 	for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++) {
 		rtable_walk(rtableid, AF_INET, NULL, mrt_rtwalk_mfcsysctl,
@@ -486,11 +485,11 @@ mrt_sysctl_mfc(void *oldp, size_t *oldlenp)
 
 	if (msa.msa_minfos != NULL && msa.msa_needed > 0 &&
 	    (error = copyout(msa.msa_minfos, oldp, msa.msa_needed)) != 0) {
-		free(msa.msa_minfos, M_TEMP, *oldlenp);
+		free(msa.msa_minfos, M_TEMP, msa.msa_len);
 		return (error);
 	}
 
-	free(msa.msa_minfos, M_TEMP, *oldlenp);
+	free(msa.msa_minfos, M_TEMP, msa.msa_len);
 	*oldlenp = msa.msa_needed;
 
 	return (0);
diff --git a/sys/netinet6/ip6_mroute.c b/sys/netinet6/ip6_mroute.c
index 9af9ed593d1..bf700ec321e 100644
--- a/sys/netinet6/ip6_mroute.c
+++ b/sys/netinet6/ip6_mroute.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip6_mroute.c,v 1.140 2024/02/11 18:14:27 mvs Exp $	*/
+/*	$OpenBSD: ip6_mroute.c,v 1.141 2024/04/06 14:23:27 bluhm Exp $	*/
 /*	$NetBSD: ip6_mroute.c,v 1.59 2003/12/10 09:28:38 itojun Exp $	*/
 /*	$KAME: ip6_mroute.c,v 1.45 2001/03/25 08:38:51 itojun Exp $	*/
 
@@ -406,8 +406,9 @@ mrt6_rtwalk_mf6csysctl(struct rtentry *rt, void *arg, unsigned int rtableid)
 	}
 
 	for (minfo = msa->ms6a_minfos;
-	     (uint8_t *)minfo < ((uint8_t *)msa->ms6a_minfos + msa->ms6a_len);
-	     minfo++) {
+	    (uint8_t *)(minfo + 1) <=
+	    (uint8_t *)msa->ms6a_minfos + msa->ms6a_len;
+	    minfo++) {
 		/* Find a new entry or update old entry. */
 		if (!IN6_ARE_ADDR_EQUAL(&minfo->mf6c_origin.sin6_addr,
 		    &satosin6(rt->rt_gateway)->sin6_addr) ||
@@ -449,13 +450,11 @@ mrt6_sysctl_mfc(void *oldp, size_t *oldlenp)
 	if (oldp != NULL && *oldlenp > MAXPHYS)
 		return EINVAL;
 
-	if (oldp != NULL)
+	memset(&msa, 0, sizeof(msa));
+	if (oldp != NULL && *oldlenp > 0) {
 		msa.ms6a_minfos = malloc(*oldlenp, M_TEMP, M_WAITOK | M_ZERO);
-	else
-		msa.ms6a_minfos = NULL;
-
-	msa.ms6a_len = *oldlenp;
-	msa.ms6a_needed = 0;
+		msa.ms6a_len = *oldlenp;
+	}
 
 	for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++) {
 		rtable_walk(rtableid, AF_INET6, NULL, mrt6_rtwalk_mf6csysctl,
@@ -464,11 +463,11 @@ mrt6_sysctl_mfc(void *oldp, size_t *oldlenp)
 
 	if (msa.ms6a_minfos != NULL && msa.ms6a_needed > 0 &&
 	    (error = copyout(msa.ms6a_minfos, oldp, msa.ms6a_needed)) != 0) {
-		free(msa.ms6a_minfos, M_TEMP, *oldlenp);
+		free(msa.ms6a_minfos, M_TEMP, msa.ms6a_len);
 		return error;
 	}
 
-	free(msa.ms6a_minfos, M_TEMP, *oldlenp);
+	free(msa.ms6a_minfos, M_TEMP, msa.ms6a_len);
 	*oldlenp = msa.ms6a_needed;
 
 	return 0;