sctp: validate from_addr_param return

[ Upstream commit 0c5dc070ff3d6246d22ddd931f23a6266249e3db ]

Ilja reported that, simply putting it, nothing was validating that
from_addr_param functions were operating on initialized memory. That is,
the parameter itself was being validated by sctp_walk_params, but it
doesn't check for types and their specific sizes and it could be a 0-length
one, causing from_addr_param to potentially work over the next parameter or
even uninitialized memory.

The fix here is to, in all calls to from_addr_param, check if enough space
is there for the wanted IP address type.

Mot-CRs-fixed: (CR)
CVE-Fixed: CVE-2021-3655
Bug: 197154735

Change-Id: Iac6ddb6a9de94e37660e286482a574824e4b6298
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Gajjala Chakradhar <gajjalac@motorola.com>
Reviewed-on: https://gerrit.mot.com/2197701
SME-Granted: SME Approvals Granted
SLTApproved: Slta Waiver
Tested-by: Jira Key
Reviewed-by: Xiangpo Zhao <zhaoxp3@motorola.com>
Submit-Approved: Jira Key
(cherry picked from commit ff64c7b4dcf2349ed9fc0ec5aa8865a3921f2fa4)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 94c775773f58c95156c6986e306b55b3e4862307..6774974a036778f1f648fe3e51de2618352e8e3b 100644 (file)
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -441,7 +441,7 @@ struct sctp_af {
                                         int saddr);
        void            (*from_sk)      (union sctp_addr *,
                                         struct sock *sk);
-       void            (*from_addr_param) (union sctp_addr *,
+       bool            (*from_addr_param) (union sctp_addr *,
                                            union sctp_addr_param *,
                                            __be16 port, int iif);
        int             (*to_addr_param) (const union sctp_addr *,

diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
index 7df3704982f547eda452cac2131afae81a5c7e9e..b9ba51b031a8f290e2da5fcbd64b3bd8b184deb7 100644 (file)
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -285,22 +285,19 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list,
                rawaddr = (union sctp_addr_param *)raw_addr_list;
 
                af = sctp_get_af_specific(param_type2af(param->type));
-               if (unlikely(!af)) {
+               if (unlikely(!af) ||
+                   !af->from_addr_param(&addr, rawaddr, htons(port), 0)) {
                        retval = -EINVAL;
-                       sctp_bind_addr_clean(bp);
-                       break;
+                       goto out_err;
                }
 
-               af->from_addr_param(&addr, rawaddr, htons(port), 0);
                if (sctp_bind_addr_state(bp, &addr) != -1)
                        goto next;
                retval = sctp_add_bind_addr(bp, &addr, sizeof(addr),
                                            SCTP_ADDR_SRC, gfp);
-               if (retval) {
+               if (retval)
                        /* Can't finish building the list, clean up. */
-                       sctp_bind_addr_clean(bp);
-                       break;
-               }
+                       goto out_err;
 
 next:
                len = ntohs(param->length);
@@ -309,6 +306,12 @@ next:
        }
 
        return retval;
+
+out_err:
+       if (retval)
+               sctp_bind_addr_clean(bp);
+
+       return retval;
 }
 
 /********************************************************************

diff --git a/net/sctp/input.c b/net/sctp/input.c
index 0247cc432e0293f2881a594367e79c666e9b9f4d..5cc8c9122c1a4357224426860aade5181b370271 100644 (file)
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -1081,7 +1081,8 @@ static struct sctp_association *__sctp_rcv_init_lookup(struct net *net,
                if (!af)
                        continue;
 
-               af->from_addr_param(paddr, params.addr, sh->source, 0);
+               if (!af->from_addr_param(paddr, params.addr, sh->source, 0))
+                       continue;
 
                asoc = __sctp_lookup_association(net, laddr, paddr, transportp);
                if (asoc)
@@ -1124,7 +1125,8 @@ static struct sctp_association *__sctp_rcv_asconf_lookup(
        if (unlikely(!af))
                return NULL;
 
-       af->from_addr_param(&paddr, param, peer_port, 0);
+       if (af->from_addr_param(&paddr, param, peer_port, 0))
+               return NULL;
 
        return __sctp_lookup_association(net, laddr, &paddr, transportp);
 }

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 7eb06fa757307912646dac1755bbb97acdb015c4..8fd13bb1c8a65b5b718b8289a6babdaf42dc21b7 100644 (file)
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -482,15 +482,20 @@ static void sctp_v6_to_sk_daddr(union sctp_addr *addr, struct sock *sk)
 }
 
 /* Initialize a sctp_addr from an address parameter. */
-static void sctp_v6_from_addr_param(union sctp_addr *addr,
+static bool sctp_v6_from_addr_param(union sctp_addr *addr,
                                    union sctp_addr_param *param,
                                    __be16 port, int iif)
 {
+       if (ntohs(param->v6.param_hdr.length) < sizeof(struct sctp_ipv6addr_param))
+               return false;
+
        addr->v6.sin6_family = AF_INET6;
        addr->v6.sin6_port = port;
        addr->v6.sin6_flowinfo = 0; /* BUG */
        addr->v6.sin6_addr = param->v6.addr;
        addr->v6.sin6_scope_id = iif;
+
+       return true;
 }
 
 /* Initialize an address parameter from a sctp_addr and return the length

diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index a7529aca2ac82782590533ffad5c5705186ad978..1d7e7a91eab0eeb0b64529cd84e4941c3b2c5f32 100644 (file)
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -276,13 +276,18 @@ static void sctp_v4_to_sk_daddr(union sctp_addr *addr, struct sock *sk)
 }
 
 /* Initialize a sctp_addr from an address parameter. */
-static void sctp_v4_from_addr_param(union sctp_addr *addr,
+static bool sctp_v4_from_addr_param(union sctp_addr *addr,
                                    union sctp_addr_param *param,
                                    __be16 port, int iif)
 {
+       if (ntohs(param->v4.param_hdr.length) < sizeof(struct sctp_ipv4addr_param))
+               return false;
+
        addr->v4.sin_family = AF_INET;
        addr->v4.sin_port = port;
        addr->v4.sin_addr.s_addr = param->v4.addr.s_addr;
+
+       return true;
 }
 
 /* Initialize an address parameter from a sctp_addr and return the length

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index e4a400f881683718708dc6bbf7261260172f5fe7..2c08246ea57a6a39f9a185ca48fae34c43da45ea 100644 (file)
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2338,11 +2338,13 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
 
        /* Process the initialization parameters.  */
        sctp_walk_params(param, peer_init, init_hdr.params) {
-               if (!src_match && (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
-                   param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
+               if (!src_match &&
+                   (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
+                    param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
                        af = sctp_get_af_specific(param_type2af(param.p->type));
-                       af->from_addr_param(&addr, param.addr,
-                                           chunk->sctp_hdr->source, 0);
+                       if (!af->from_addr_param(&addr, param.addr,
+                                                chunk->sctp_hdr->source, 0))
+                               continue;
                        if (sctp_cmp_addr_exact(sctp_source(chunk), &addr))
                                src_match = 1;
                }
@@ -2528,7 +2530,8 @@ static int sctp_process_param(struct sctp_association *asoc,
                        break;
 do_addr_param:
                af = sctp_get_af_specific(param_type2af(param.p->type));
-               af->from_addr_param(&addr, param.addr, htons(asoc->peer.port), 0);
+               if (!af->from_addr_param(&addr, param.addr, htons(asoc->peer.port), 0))
+                       break;
                scope = sctp_scope(peer_addr);
                if (sctp_in_scope(net, &addr, scope))
                        if (!sctp_assoc_add_peer(asoc, &addr, gfp, SCTP_UNCONFIRMED))
@@ -2621,15 +2624,13 @@ do_addr_param:
                addr_param = param.v + sizeof(struct sctp_addip_param);
 
                af = sctp_get_af_specific(param_type2af(addr_param->p.type));
-               if (af == NULL)
+               if (!af)
                        break;
 
-               af->from_addr_param(&addr, addr_param,
-                                   htons(asoc->peer.port), 0);
+               if (!af->from_addr_param(&addr, addr_param,
+                                        htons(asoc->peer.port), 0))
+                       break;
 
-               /* if the address is invalid, we can't process it.
-                * XXX: see spec for what to do.
-                */
                if (!af->addr_valid(&addr, NULL, NULL))
                        break;
 
@@ -3040,7 +3041,8 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
        if (unlikely(!af))
                return SCTP_ERROR_DNS_FAILED;
 
-       af->from_addr_param(&addr, addr_param, htons(asoc->peer.port), 0);
+       if (!af->from_addr_param(&addr, addr_param, htons(asoc->peer.port), 0))
+               return SCTP_ERROR_DNS_FAILED;
 
        /* ADDIP 4.2.1  This parameter MUST NOT contain a broadcast
         * or multicast address.
@@ -3305,7 +3307,8 @@ static void sctp_asconf_param_success(struct sctp_association *asoc,
 
        /* We have checked the packet before, so we do not check again. */
        af = sctp_get_af_specific(param_type2af(addr_param->p.type));
-       af->from_addr_param(&addr, addr_param, htons(bp->port), 0);
+       if (!af->from_addr_param(&addr, addr_param, htons(bp->port), 0))
+               return;
 
        switch (asconf_param->param_hdr.type) {
        case SCTP_PARAM_ADD_IP: