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 3e9b00e..1913b17 100644 (file)
--- 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 9af9ed5..bf700ec 100644 (file)
--- 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;