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

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

[Xingyuan Mo]

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. peer_addr is validated in
place, so a v4-mapped v6 address is normalised to AF_INET before the
association and its primary transport are built from it.

For that, sctp_v6_addr_valid() has to return 0 for a non-v4-mapped
address when it is called with a socket that is not PF_INET6, so that
IPv6 addresses are never used on IPv4 sockets. The check is done after
the v4-mapped handling, so v4-mapped addresses passed to an IPv4
socket through the socket API keep being accepted. 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 <hdthky0@gmail.com>
---
v2: https://lore.kernel.org/netdev/20260913113522.2674588-1-hdthky0@gmail.com/
- 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)

v3: https://lore.kernel.org/netdev/20260915212917.3775248-1-hdthky0@gmail.com/
- drop the sctp_transport_new() check
- merge the !af checks into the conditions they guard with ||
- drop the comment above the sctp_unpack_cookie() check

v4: https://lore.kernel.org/netdev/20260917030505.4176635-1-hdthky0@gmail.com/
- move the non-PF_INET6 socket check after the v4-mapped handling in
  sctp_v6_addr_valid()
- validate bear_cookie->peer_addr in place instead of a copy

 net/sctp/ipv6.c          |  6 +++++-
 net/sctp/sm_make_chunk.c | 23 ++++++++++++-----------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index ef26878f1282..97390569c4bd 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -725,7 +725,8 @@ static int sctp_v6_available(union sctp_addr *addr, struct sctp_sock *sp)
  * SCTP.
  *
  * Output:
- * Return 0 - If the address is a non-unicast or an illegal address.
+ * Return 0 - If the address is a non-unicast or an illegal address,
+ *            or a non-v4-mapped address on a non-PF_INET6 socket.
  * Return 1 - If the address is a unicast.
  */
 static int sctp_v6_addr_valid(union sctp_addr *addr,
@@ -749,6 +750,9 @@ static int sctp_v6_addr_valid(union sctp_addr *addr,
 	if (!(ret & IPV6_ADDR_UNICAST))
 		return 0;
 
+	if (sp && sctp_opt2sk(sp)->sk_family != PF_INET6)
+		return 0;
+
 	return 1;
 }
 
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 84a4c97d0f75..175dbb242e1c 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,11 @@ struct sctp_association *sctp_unpack_cookie(
 		goto fail;
 	}
 
+	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,8 +2388,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->from_addr_param(&addr, param.addr,
-						 chunk->sctp_hdr->source, 0))
+			if (!af || !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;
@@ -2554,17 +2561,11 @@ 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->from_addr_param(&addr, param.addr, htons(asoc->peer.port), 0))
+		if (!af || !af->from_addr_param(&addr, param.addr,
+						htons(asoc->peer.port), 0) ||
+		    !af->addr_valid(&addr, sctp_sk(asoc->base.sk), NULL))
 			break;
 		scope = sctp_scope(peer_addr);
 		if (sctp_in_scope(net, &addr, scope))
-- 
2.43.0

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch now validates peer_addr in place.

Pre-existing issues:
- [Low] Two comments next to the code this patch changes contradict the
  implementation.

--

sctp: reject forged cookie peer_addr with unknown address family

This patch validates the COOKIE-ECHO peer_addr with af->addr_valid() in
sctp_unpack_cookie(). Bad families are rejected, and v4-mapped addresses
are normalised in place. It also makes sctp_v6_addr_valid() reject
non-v4-mapped addresses on non-PF_INET6 sockets, uses addr_valid() to
simplify sctp_process_param(), and adds a missing !af check in
sctp_process_init().

> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index ef26878f1282a..97390569c4bdc 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -725,7 +725,8 @@ static int sctp_v6_available(union sctp_addr *addr, struct sctp_sock *sp)
>   * SCTP.
>   *
>   * Output:
> - * Return 0 - If the address is a non-unicast or an illegal address.
> + * Return 0 - If the address is a non-unicast or an illegal address,
> + *            or a non-v4-mapped address on a non-PF_INET6 socket.
>   * Return 1 - If the address is a unicast.
>   */
>  static int sctp_v6_addr_valid(union sctp_addr *addr,

[Severity: Low]

This isn't a bug introduced by this patch, but two nearby comments
don't match the code that this patch now relies on.

The v4-mapped branch of sctp_v6_addr_valid() says:

	/* Note: This routine is used in input, so v4-mapped-v6
	 * are disallowed here when there is no sctp_sock.
	 */
	if (sp && ipv6_only_sock(sctp_opt2sk(sp)))
		return 0;
	sctp_v6_map_v4(addr);

When sp is NULL, as in the sctp_rcv() input path, the code maps and
accepts the address instead of rejecting it.

The header comment of sctp_v4_addr_valid() in net/sctp/protocol.c only
lists non-unicast or illegal addresses as reasons to return 0. The
function also returns 0 for any IPv4 address on an IPV6_V6ONLY socket:

	/* IPv4 addresses not allowed */
	if (sp && ipv6_only_sock(sctp_opt2sk(sp)))
		return 0;

This patch removes the explicit ipv6_only_sock() test from
sctp_process_param(), so that function now depends on this check.

The sctp_v6_addr_valid() header comment is already being updated here.
Could these two comments be updated as well?

[ ... ]

> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 84a4c97d0f755..175dbb242e1cb 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c

[ ... ]

> @@ -1841,6 +1843,11 @@ struct sctp_association *sctp_unpack_cookie(
>  		goto fail;
>  	}
>  
> +	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;
> +

[Severity: Medium]

Can this modify packet data that is shared with a clone? bear_cookie
points into the received skb. sctp_sf_do_5_1D_ce() and
sctp_sf_do_5_2_4_dupcook() both set:

	chunk->subh.cookie_hdr =
		(struct sctp_signed_cookie *)chunk->skb->data;

For an AF_INET6 v4-mapped peer_addr, sctp_v6_addr_valid() calls
sctp_v6_map_v4(), which writes directly into that buffer:

	addr->v4.sin_family = AF_INET;
	addr->v4.sin_port = addr->v6.sin6_port;
	addr->v4.sin_addr.s_addr = addr->v6.sin6_addr.s6_addr32[3];

No skb_cloned() or skb_unshare() check happens before this write.

If an AUTH chunk is bundled before the COOKIE-ECHO,
sctp_endpoint_bh_rcv() and sctp_assoc_bh_rcv() do:

	chunk->auth_chunk = skb_clone(chunk->skb,
				GFP_ATOMIC);

The clone shares the same data.

sctp_sf_do_5_1D_ce() calls sctp_unpack_cookie() first. AUTH is verified
only afterwards:

sctp_auth_chunk_verify()
  sctp_sf_authenticate()
    sctp_auth_calculate_hmac()
      data_len = skb_tail_pointer(skb) - (unsigned char *)auth;

That HMAC covers the COOKIE-ECHO body, including the rewritten
peer_addr. The peer computed its HMAC over the original bytes. Would
verification then fail and the COOKIE-ECHO be dropped through
sctp_sf_pdiscard()? The duplicate-cookie handlers use the same order:
unpack first, then verify AUTH.

Other holders of a clone, such as raw IPPROTO_SCTP sockets and non-mmap
AF_PACKET readers, would also see the changed bytes. sctp_rcv() doesn't
prevent this, because skb_linearize() does nothing to an skb that is
already linear but cloned.

A v4-mapped peer_addr doesn't have to be forged. For an INIT that
arrives over IPv6 from a ::ffff:a.b.c.d source, sctp_make_temp_asoc()
stores the raw source in the signed cookie:

	SCTP_INPUT_CB(skb)->af->from_skb(&asoc->c.peer_addr, skb, 1);

It can also be forged directly when cookie authentication is disabled.

In that case, the commit message statement "peer_addr is validated in
place, so a v4-mapped v6 address is normalised to AF_INET before the
association and its primary transport are built from it" would not hold
when AUTH is bundled.

Would it work to validate and normalise a stack copy of peer_addr, and
pass that copy to the association and transport setup, instead of
writing into the packet buffer?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009074601.3954275-1-hdthky0%40gmail.com