Thread (3 messages) flat view 3 messages, 3 authors, 2d ago

Re: [PATCH v2] sctp: reject forged cookie peer_addr with unknown address family

From: Xin Long <lucien.xin@gmail.com>
Date: 2026-09-16 01:02:42
Also in: linux-sctp

On Tue, Sep 15, 2026 at 5:29 PM Xingyuan Mo [off-list ref] wrote:
quoted hunk ↗ jump to hunk
When cookie authentication is disabled, COOKIE-ECHO peer_addr is
attacker-controlled. An invalid sa_family made sctp_get_af_specific()
return NULL and crash in sctp_transport_init() on
af_specific->sockaddr_len. Validate it with af->addr_valid() in
sctp_unpack_cookie(), which also rejects a mismatched family, e.g.
an AF_INET6 peer_addr on an IPv4 socket that would later crash in
sctp_v6_get_dst() on inet6_sk(sk)->opt, and bail in
sctp_transport_new() if af_specific is missing. Validate a copy of
peer_addr, since addr_valid() rewrites a v4-mapped v6 address in
place and the cookie still lives in the received skb, which is not
guaranteed to be unshared.

For that, sctp_v6_addr_valid() has to return 0 when it is called with
a socket that is not PF_INET6, so that IPv6 addresses are never used
on IPv4 sockets. As addr_valid() now also checks sk_family and
ipv6_only_sock, simplify the address handling in sctp_process_param()
with it, as it additionally filters out non-unicast addresses. As a
side effect, a v4-mapped address in an IPv6 address parameter is now
normalised to AF_INET before the transport is created, matching what
the socket API paths already do.

Also add the missing !af check in sctp_process_init(), as
sctp_get_af_specific(AF_INET6) returns NULL on CONFIG_IPV6=n builds.

  BUG: KASAN: null-ptr-deref in sctp_transport_new+0xa7/0x350
  Read of size 4 at addr 00000000000000b4 by task poc/682
  Call Trace:
   <IRQ>
   sctp_transport_new+0xa7/0x350
   sctp_assoc_add_peer+0x153/0x850
   sctp_process_init+0xf9/0x1180
   sctp_sf_do_5_1D_ce+0x464/0xbc0
   sctp_do_sm+0x114/0x2990
   sctp_endpoint_bh_rcv+0x280/0x430
   sctp_inq_push+0xdd/0x100
   sctp_rcv+0x17f5/0x1ae0
   sctp4_rcv+0x2b/0x40
   ip_protocol_deliver_rcu+0x25b/0x270
   ip_local_deliver+0xd1/0xe0
   </IRQ>
  Kernel panic - not syncing: Fatal exception in interrupt

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: opencode:deepseek-v4
Signed-off-by: Xingyuan Mo <redacted>
---

Changes since v1 [1]:
- validate the forged cookie peer_addr with af->addr_valid() instead of
  a family whitelist, and make sctp_v6_addr_valid() reject non-PF_INET6
  sockets, so a forged AF_INET6 cookie can no longer crash an IPv4
  socket in sctp_v6_get_dst() (found by Sashiko)
- simplify sctp_process_param() address handling with addr_valid(), as
  addr_valid() now also checks sk_family and ipv6_only_sock
- add the missing !af check in sctp_process_init() for CONFIG_IPV6=n
  builds (suggested by Xin Long)

[1] https://lore.kernel.org/netdev/20260913113522.2674588-1-hdthky0@gmail.com/ (local)

 net/sctp/ipv6.c          |  3 +++
 net/sctp/sm_make_chunk.c | 25 +++++++++++++++++--------
 net/sctp/transport.c     |  3 +++
 3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index ef26878f1282..c45ac15f3b79 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -734,6 +734,9 @@ static int sctp_v6_addr_valid(union sctp_addr *addr,
 {
        int ret = ipv6_addr_type(&addr->v6.sin6_addr);

+       if (sp && sctp_opt2sk(sp)->sk_family != PF_INET6)
+               return 0;
+
        /* Support v4-mapped-v6 address. */
        if (ret == IPV6_ADDR_MAPPED) {
                /* Note: This routine is used in input, so v4-mapped-v6
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 84a4c97d0f75..cfc4973f4d4a 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1732,7 +1732,9 @@ struct sctp_association *sctp_unpack_cookie(
        struct sctp_cookie *bear_cookie;
        struct sctp_chunkhdr *ch;
        unsigned int len, chlen;
+       union sctp_addr paddr;
        enum sctp_scope scope;
+       struct sctp_af *af;
        ktime_t kt;

        /* Header size is static data prior to the actual cookie, including
@@ -1841,6 +1843,15 @@ struct sctp_association *sctp_unpack_cookie(
                goto fail;
        }

+       /* peer_addr is peer-controlled when cookie authentication is
+        * disabled.  Validate a copy, as addr_valid() may rewrite a
+        * v4-mapped address in place.
+        */
af->addr_valid() is more like ensuring the address is a unicast address
and the socket supports the address family. You can either improve
the inline comment above or just remove it.
quoted hunk ↗ jump to hunk
+       paddr = bear_cookie->peer_addr;
+       af = sctp_get_af_specific(paddr.sa.sa_family);
+       if (!af || !af->addr_valid(&paddr, sctp_sk(ep->base.sk), NULL))
+               goto malformed;
+
        /* Make a new base association.  */
        scope = sctp_scope(sctp_source(chunk));
        retval = sctp_association_new(ep, ep->base.sk, scope, gfp);
@@ -2381,6 +2392,8 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
                    (param.p->type == SCTP_PARAM_IPV4_ADDRESS ||
                     param.p->type == SCTP_PARAM_IPV6_ADDRESS)) {
                        af = sctp_get_af_specific(param_type2af(param.p->type));
+                       if (!af)
+                               continue;
                        if (!af->from_addr_param(&addr, param.addr,
                                                 chunk->sctp_hdr->source, 0))
                                continue;
You can merge these 2 checks with (!af || !af->from_addr_param()).
quoted hunk ↗ jump to hunk
@@ -2554,18 +2567,14 @@ static int sctp_process_param(struct sctp_association *asoc,
         */
        switch (param.p->type) {
        case SCTP_PARAM_IPV6_ADDRESS:
-               if (PF_INET6 != asoc->base.sk->sk_family)
-                       break;
-               goto do_addr_param;
-
        case SCTP_PARAM_IPV4_ADDRESS:
-               /* v4 addresses are not allowed on v6-only socket */
-               if (ipv6_only_sock(asoc->base.sk))
-                       break;
-do_addr_param:
                af = sctp_get_af_specific(param_type2af(param.p->type));
+               if (!af)
+                       break;
                if (!af->from_addr_param(&addr, param.addr, htons(asoc->peer.port), 0))
                        break;
Same here.
quoted hunk ↗ jump to hunk
+               if (!af->addr_valid(&addr, sctp_sk(asoc->base.sk), NULL))
+                       break;
                scope = sctp_scope(peer_addr);
                if (sctp_in_scope(net, &addr, scope))
                        if (!sctp_assoc_add_peer(asoc, &addr, gfp, SCTP_UNCONFIRMED))
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 6ea55b9fbde4..cd1a604d6f8e 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -92,6 +92,9 @@ struct sctp_transport *sctp_transport_new(struct net *net,
 {
        struct sctp_transport *transport;

+       if (!sctp_get_af_specific(addr->sa.sa_family))
+               return NULL;
+
Any reason why we need this check?

Thanks.
        transport = kzalloc_obj(*transport, gfp);
        if (!transport)
                return NULL;
--
2.43.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help