Re: [PATCH net 1/1] net: sctp: fix AUTH HMAC list overflow into auth_chunks

Xin Long <[email protected]> Fri, 3 Jul 2026 16:20:14 -0400
Newsgroups org.kernel.vger.linux-sctp,org.kernel.vger.netdev
Message-ID <CADvbK_fsEC=dViaQMDVD4BQY_i=Yv=GHfyQdFkHxSVeannjjhg@mail.gmail.com>
On Fri, Jul 3, 2026 at 3:19 AM Ren Wei <[email protected]> wrote:
>
> From: Zihan Xi <[email protected]>
>
> sctp_auth_ep_set_hmacs() may advertise a 12-byte HMAC-ALGO parameter when
> four identifiers are configured, but the association only stores ten bytes
> in c.auth_hmacs. sctp_association_init() copies the advertised length and
> overwrites the adjacent auth_chunks field, so sctp_auth_asoc_verify_hmac_id()
> accepts forged HMAC identifiers and sctp_auth_get_hmac() indexes past
> sctp_hmac_list.
>
> Clamp the stored parameter length to the association buffer, copy only that
> many bytes when initializing an association, and reject out-of-range HMAC
> identifiers in sctp_auth_get_hmac().
>
> Fixes: 65b07e5d0d09 ("[SCTP]: API updates to suport SCTP-AUTH extensions.")
> Cc: [email protected]
> Reported-by: Yuan Tan <[email protected]>
> Reported-by: Xin Liu <[email protected]>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zihan Xi <[email protected]>
> Reviewed-by: Ren Wei <[email protected]>
> ---
>  net/sctp/associola.c    | 10 +++++++---
>  net/sctp/auth.c         | 10 ++++++++--
>  net/sctp/sm_statefuns.c |  2 ++
>  3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 62d3cc1558..760457def6 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -260,9 +260,13 @@ static struct sctp_association *sctp_association_init(
>         asoc->strreset_enable = ep->strreset_enable;
>
>         /* Save the hmacs and chunks list into this association */
> -       if (ep->auth_hmacs_list)
> -               memcpy(asoc->c.auth_hmacs, ep->auth_hmacs_list,
> -                       ntohs(ep->auth_hmacs_list->param_hdr.length));
> +       if (ep->auth_hmacs_list) {
> +               size_t hmac_len = min_t(size_t,
> +                               ntohs(ep->auth_hmacs_list->param_hdr.length),
> +                               sizeof(asoc->c.auth_hmacs));
> +
> +               memcpy(asoc->c.auth_hmacs, ep->auth_hmacs_list, hmac_len);
> +       }
>         if (ep->auth_chunk_list)
>                 memcpy(asoc->c.auth_chunks, ep->auth_chunk_list,
>                         ntohs(ep->auth_chunk_list->param_hdr.length));
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index be9782760f..4d14bd6185 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -447,6 +447,8 @@ struct sctp_shared_key *sctp_auth_get_shkey(
>
>  const struct sctp_hmac *sctp_auth_get_hmac(__u16 hmac_id)
>  {
> +       if (hmac_id >= SCTP_AUTH_NUM_HMACS)
> +               return NULL;
>         return &sctp_hmac_list[hmac_id];
>  }
>
> @@ -510,6 +512,9 @@ int sctp_auth_asoc_verify_hmac_id(const struct sctp_association *asoc,
>         hmacs = (struct sctp_hmac_algo_param *)asoc->c.auth_hmacs;
>         n_elt = (ntohs(hmacs->param_hdr.length) -
>                  sizeof(struct sctp_paramhdr)) >> 1;
> +       n_elt = min_t(__u16, n_elt,
> +                     (sizeof(asoc->c.auth_hmacs) -
> +                      sizeof(struct sctp_paramhdr)) / sizeof(__u16));
>
>         return __sctp_auth_find_hmacid(hmacs->hmac_ids, n_elt, hmac_id);
>  }
> @@ -708,8 +713,9 @@ int sctp_auth_ep_set_hmacs(struct sctp_endpoint *ep,
>                 ep->auth_hmacs_list->hmac_ids[i] =
>                                 htons(hmacs->shmac_idents[i]);
>         ep->auth_hmacs_list->param_hdr.length =
> -                       htons(sizeof(struct sctp_paramhdr) +
> -                       hmacs->shmac_num_idents * sizeof(__u16));
> +                       htons(min_t(__u16, sizeof(struct sctp_paramhdr) +
> +                                     hmacs->shmac_num_idents * sizeof(__u16),
> +                             SCTP_AUTH_NUM_HMACS * sizeof(__u16) + 2));
>         return 0;
>  }
>
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index d23d935e12..21cda509a0 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -4431,6 +4431,8 @@ static enum sctp_ierror sctp_sf_authenticate(
>         sig_len = ntohs(chunk->chunk_hdr->length) -
>                   sizeof(struct sctp_auth_chunk);
>         hmac = sctp_auth_get_hmac(ntohs(auth_hdr->hmac_id));
> +       if (!hmac)
> +               return SCTP_IERROR_AUTH_BAD_HMAC;
>         if (sig_len != hmac->hmac_len)
>                 return SCTP_IERROR_PROTO_VIOLATION;
>
> --
> 2.43.0
I think real issue is the member:

  __u8 auth_hmacs[SCTP_AUTH_NUM_HMACS * sizeof(__u16) + 2]

in  struct sctp_cookie.

It's supposed to include a 'struct struct sctp_paramhdr' + N * hmac_id.
However, sizeof(struct struct sctp_paramhdr) is 4 not 2, which causes
the overflow when copying it from ep to asoc.

The right fix should be:

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index affee44bd38e..cccc662561aa 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -312,7 +312,8 @@ struct sctp_cookie {

        __u8 auth_random[sizeof(struct sctp_paramhdr) +
                         SCTP_AUTH_RANDOM_LENGTH];
-       __u8 auth_hmacs[SCTP_AUTH_NUM_HMACS * sizeof(__u16) + 2];
+       __u8 auth_hmacs[sizeof(struct sctp_paramhdr) +
+                       SCTP_AUTH_NUM_HMACS * sizeof(__u16)];
        __u8 auth_chunks[sizeof(struct sctp_paramhdr) + SCTP_AUTH_MAX_CHUNKS];

        /* This is a shim for my peer's INIT packet, followed by

Please give it a try in your test env, Thanks.