[PATCH net] sctp: validate cached peer INIT chunk length in COOKIE_ECHO processing

[PATCH net] sctp: validate cached peer INIT chunk length in COOKIE_ECHO processing

[Xin Long]

When a listening SCTP server processes a COOKIE_ECHO chunk, the cached
peer INIT chunk embedded after the cookie is parsed and its parameters
are later walked by sctp_process_init() using sctp_walk_params().

However, the chunk header length of this cached INIT chunk was not
validated against the remaining buffer in the COOKIE_ECHO payload. If
the length field is inflated, the parameter walk can run beyond the
actual received data, leading to out-of-bounds reads and potential
memory corruption during later parameter handling (e.g. STATE_COOKIE
processing and kmemdup() copies).

Add a bounds check in sctp_unpack_cookie() to ensure the cached INIT
chunk length does not exceed the available data in the COOKIE_ECHO
buffer before it is used.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Brian Geffon <bgeffon@google.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_make_chunk.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index de86ac088289..85264862fb6b 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1730,6 +1730,7 @@ struct sctp_association *sctp_unpack_cookie(
 	struct sctp_signed_cookie *cookie;
 	struct sk_buff *skb = chunk->skb;
 	struct sctp_cookie *bear_cookie;
+	struct sctp_chunkhdr *ch;
 	enum sctp_scope scope;
 	unsigned int len;
 	ktime_t kt;
@@ -1759,6 +1760,10 @@ struct sctp_association *sctp_unpack_cookie(
 	cookie = chunk->subh.cookie_hdr;
 	bear_cookie = &cookie->c;
 
+	ch = (struct sctp_chunkhdr *)(bear_cookie + 1);
+	if (ntohs(ch->length) > len - fixed_size)
+		goto malformed;
+
 	/* Verify the cookie's MAC, if cookie authentication is enabled. */
 	if (sctp_sk(ep->base.sk)->cookie_auth_enable) {
 		u8 mac[SHA256_DIGEST_SIZE];
-- 
2.47.1

Re: [PATCH net] sctp: validate cached peer INIT chunk length in COOKIE_ECHO processing

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
sctp: validate cached peer INIT chunk length in COOKIE_ECHO processing

This commit adds a bounds check to ensure the cached peer INIT chunk
length does not exceed the available data in the COOKIE_ECHO buffer
during unpacking.

Simon says:

Hi Xin,

Although the issues flagged below are flagged as pre-existing I am
wondering if they directly impact the effectiveness of this patch.
Could you take a look?

> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index de86ac088289e..85264862fb6b6 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c

[ ... ]

> @@ -1759,6 +1760,10 @@ struct sctp_association *sctp_unpack_cookie(
>  	cookie = chunk->subh.cookie_hdr;
>  	bear_cookie = &cookie->c;
>  
> +	ch = (struct sctp_chunkhdr *)(bear_cookie + 1);
> +	if (ntohs(ch->length) > len - fixed_size)
> +		goto malformed;
> +

[Severity: High]
This isn't a bug introduced by this patch, but does this check also
need to ensure the length is at least the size of the INIT header?

If ch->length is small, for example 4 bytes for just the chunk header,
this check passes. However, sctp_process_init() appears to unconditionally
read the init_hdr fields:

net/sctp/sm_make_chunk.c:sctp_process_init() {
    ...
    asoc->peer.i.init_tag = ntohl(peer_init->init_hdr.init_tag);
    asoc->peer.i.a_rwnd = ntohl(peer_init->init_hdr.a_rwnd);
    ...
}

Could an attacker send a COOKIE_ECHO chunk with an embedded INIT chunk
truncated to 4 bytes and trigger an out-of-bounds read past the SKB payload?

[Severity: High]
This is a pre-existing issue, but is cookie->raw_addr_list_len properly
validated against the remaining payload length before use?

If SCTP cookie authentication is disabled, an unvalidated
raw_addr_list_len could lead to out-of-bounds reads when
sctp_assoc_set_bind_addr_from_cookie() calls sctp_raw_to_bind_addrs()
to parse the bind addresses:

net/sctp/bind_addr.c:sctp_raw_to_bind_addrs() {
    ...
    while (addrs_len >= sizeof(struct sctp_paramhdr)) {
        param = (struct sctp_paramhdr *)raw_addr_list;
        len = ntohs(param->length);
        addrs_len -= len;
        raw_addr_list += len;
    }
    ...
}

If the unvalidated addrs_len is artificially large, or if a supplied
param->length is larger than addrs_len causing an integer underflow on
addrs_len -= len, could this bypass the loop condition, read into unmapped
pages, and cause a panic?

>  	/* Verify the cookie's MAC, if cookie authentication is enabled. */
>  	if (sctp_sk(ep->base.sk)->cookie_auth_enable) {
>  		u8 mac[SHA256_DIGEST_SIZE];

Re: [PATCH net] sctp: validate cached peer INIT chunk length in COOKIE_ECHO processing

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  1 Jun 2026 21:06:06 -0400 you wrote:
> When a listening SCTP server processes a COOKIE_ECHO chunk, the cached
> peer INIT chunk embedded after the cookie is parsed and its parameters
> are later walked by sctp_process_init() using sctp_walk_params().
> 
> However, the chunk header length of this cached INIT chunk was not
> validated against the remaining buffer in the COOKIE_ECHO payload. If
> the length field is inflated, the parameter walk can run beyond the
> actual received data, leading to out-of-bounds reads and potential
> memory corruption during later parameter handling (e.g. STATE_COOKIE
> processing and kmemdup() copies).
> 
> [...]

Here is the summary with links:
  - [net] sctp: validate cached peer INIT chunk length in COOKIE_ECHO processing
    https://git.kernel.org/netdev/net/c/0861615c28de

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net] sctp: validate cached peer INIT chunk length in COOKIE_ECHO processing

[Xin Long]

On Thu, Jun 4, 2026 at 6:48 AM Simon Horman <horms@kernel.org> wrote:
>
> From: 'Simon Horman' <horms@kernel.org>
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> sctp: validate cached peer INIT chunk length in COOKIE_ECHO processing
>
> This commit adds a bounds check to ensure the cached peer INIT chunk
> length does not exceed the available data in the COOKIE_ECHO buffer
> during unpacking.
>
> Simon says:
>
> Hi Xin,
>
> Although the issues flagged below are flagged as pre-existing I am
> wondering if they directly impact the effectiveness of this patch.
> Could you take a look?
>
> > diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> > index de86ac088289e..85264862fb6b6 100644
> > --- a/net/sctp/sm_make_chunk.c
> > +++ b/net/sctp/sm_make_chunk.c
>
> [ ... ]
>
> > @@ -1759,6 +1760,10 @@ struct sctp_association *sctp_unpack_cookie(
> >       cookie = chunk->subh.cookie_hdr;
> >       bear_cookie = &cookie->c;
> >
> > +     ch = (struct sctp_chunkhdr *)(bear_cookie + 1);
> > +     if (ntohs(ch->length) > len - fixed_size)
> > +             goto malformed;
> > +
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this check also
> need to ensure the length is at least the size of the INIT header?
>
> If ch->length is small, for example 4 bytes for just the chunk header,
> this check passes. However, sctp_process_init() appears to unconditionally
> read the init_hdr fields:
>
> net/sctp/sm_make_chunk.c:sctp_process_init() {
>     ...
>     asoc->peer.i.init_tag = ntohl(peer_init->init_hdr.init_tag);
>     asoc->peer.i.a_rwnd = ntohl(peer_init->init_hdr.a_rwnd);
>     ...
> }
>
> Could an attacker send a COOKIE_ECHO chunk with an embedded INIT chunk
> truncated to 4 bytes and trigger an out-of-bounds read past the SKB payload?
>
> [Severity: High]
> This is a pre-existing issue, but is cookie->raw_addr_list_len properly
> validated against the remaining payload length before use?
>
> If SCTP cookie authentication is disabled, an unvalidated
> raw_addr_list_len could lead to out-of-bounds reads when
> sctp_assoc_set_bind_addr_from_cookie() calls c()
> to parse the bind addresses:
>
> net/sctp/bind_addr.c:sctp_raw_to_bind_addrs() {
>     ...
>     while (addrs_len >= sizeof(struct sctp_paramhdr)) {
>         param = (struct sctp_paramhdr *)raw_addr_list;
>         len = ntohs(param->length);
>         addrs_len -= len;
>         raw_addr_list += len;
>     }
>     ...
> }
>
> If the unvalidated addrs_len is artificially large, or if a supplied
> param->length is larger than addrs_len causing an integer underflow on
> addrs_len -= len, could this bypass the loop condition, read into unmapped
> pages, and cause a panic?
>
Yes, these seem like real issues.

We likely need more cookie validation than just checking the chunk length
and raw_addr_list_len. This was probably missed because the cookie is
generated locally, but it can still be maliciously modified.

I will try to reproduce and get them fixed.

Thanks.