RE: [PATCH net] sctp: bound the auth_chunks copy length in SCTP_LOCAL_AUTH_CHUNKS

[JEAN Jeremy <Jeremy.Jean@ssi.gouv.fr>]

[-- Attachment #1: Type: text/plain, Size: 4358 bytes --]

Dear all,

Attached is a proposed fix that relies on a code suggestion from Xin Long, but reworked to better handle all cases.
Does this look ok?
I verified experimentally on 8ba098e6b6ff that this fix solves the two bugs I reported.

Regards,
Jérémy

-----Message d'origine-----
De : Xin Long <lucien.xin@gmail.com>
Envoyé : vendredi 31 juillet 2026 17:27
À : Jun Yang <juny24602@gmail.com>; JEAN Jeremy <Jeremy.Jean@ssi.gouv.fr>
Cc : netdev@vger.kernel.org; Jun Yang <junvyyang@tencent.com>; stable@kernel.org; TencentOS Corvus AI <corvus@tencent.com>; Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Simon Horman <horms@kernel.org>; Vlad Yasevich <vladislav.yasevich@hp.com>; linux-sctp@vger.kernel.org; linux-kernel@vger.kernel.org
Objet : Re: [PATCH net] sctp: bound the auth_chunks copy length in SCTP_LOCAL_AUTH_CHUNKS

On Thu, Jul 30, 2026 at 5:02 AM Jun Yang <juny24602@gmail.com> wrote:
>
> From: Jun Yang <junvyyang@tencent.com>
>
> sctp_getsockopt_local_auth_chunks() copies ntohs(length) -
> sizeof(paramhdr) bytes out of ch->chunks without bounding the count.
> For an association ch is the fixed-size asoc->c.auth_chunks[] array
> (sizeof(struct sctp_paramhdr) + SCTP_AUTH_MAX_CHUNKS bytes), whose
> length field is restored from the received state cookie by
> sctp_unpack_cookie() and is not validated against the array size.  A
> cookie carrying an oversized length makes copy_to_user() read past the
> array (out-of-bounds read).
>
> Bound the chunk count to SCTP_AUTH_MAX_CHUNKS before the copy.  That
> is the most valid chunk bytes either source can hold, so legitimate
> output is unchanged, and it also covers a stored length smaller than
> the parameter header, which would otherwise underflow.
>
> Fixes: 65b07e5d0d09 ("[SCTP]: API updates to suport SCTP-AUTH
> extensions.")
> Cc: stable@kernel.org
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Signed-off-by: Jun Yang <junvyyang@tencent.com>
> ---
>  net/sctp/socket.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c index
> 9a6da4e0d741..3e9dacb772a0 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -7109,6 +7109,12 @@ static int sctp_getsockopt_local_auth_chunks(struct sock *sk, int len,
>                 goto num;
>
>         num_chunks = ntohs(ch->param_hdr.length) - sizeof(struct
> sctp_paramhdr);
> +       /* asoc->c.auth_chunks[] holds at most SCTP_AUTH_MAX_CHUNKS bytes but its
> +        * length is restored from the state cookie and is not bounded here;
> +        * clamp so an oversized length cannot read past the array.
> +        */
> +       if (num_chunks > SCTP_AUTH_MAX_CHUNKS)
> +               num_chunks = SCTP_AUTH_MAX_CHUNKS;
>         if (len < sizeof(struct sctp_authchunks) + num_chunks)
>                 return -EINVAL;
>
> --
> 2.55.0
>
The issue requires sysctl net.sctp.cookie_hmac_alg=none, right?

If so, Jean Jeremy has reported two issues caused by the missing validation of the auth params in the cookie. We may need to validate all auth_random, auth_hmacs and auth_chunks in sctp_unpack_cookie().

Let's wait a bit and see if he proceeds with the fix I've suggested.

Thanks.
Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter contact.rgpd@sgdsn.gouv.fr. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: contact.rgpd@sgdsn.gouv.fr. If you have received this message in error, we thank you for informing the sender and destroying the message.

[-- Attachment #2: cookie-hmac-id-new.patch --]
[-- Type: application/octet-stream, Size: 6922 bytes --]

From 835c40c3aa0b1959b1fa27ed41385487987a0098 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Jean?= <Jeremy.Jean@ssi.gouv.fr>
Date: Sun, 26 Jul 2026 23:03:04 +0000
Subject: [PATCH] sctp: validate cookie AUTH state before use
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When cookie authentication is disabled, COOKIE_ECHO restores fixed-size
AUTH fields directly from peer-controlled cookie bytes.  A forged RANDOM
length, HMAC list, or CHUNKS list can then reach association consumers
with lengths or identifiers that were never validated against the local
backing arrays.

Validate the cookie's RANDOM, HMACS, and CHUNKS parameters at the cookie
trust boundary before copying them into the association.  Reject invalid
types, malformed lengths, unsupported HMAC identifiers, HMAC lists
without SHA1, and forbidden chunk ids.  Keep the downstream HMAC lookup
path range-safe as a second line of defense.

Fixes: bbd0d59809f9 ("[SCTP]: Implement the receive and verification of AUTH chunk")
Fixes: 1f485649f529 ("[SCTP]: Implement SCTP-AUTH internals")
Signed-off-by: Jérémy Jean <Jeremy.Jean@ssi.gouv.fr>
---
 include/net/sctp/auth.h  |  3 ++
 net/sctp/auth.c          | 88 ++++++++++++++++++++++++++++++++++++++++++++++++
 net/sctp/sm_make_chunk.c |  3 ++
 net/sctp/sm_statefuns.c  |  3 ++
 4 files changed, 97 insertions(+)

diff --git a/include/net/sctp/auth.h b/include/net/sctp/auth.h
index 6f2cd562b1de..74b3790e2a3d 100644
--- a/include/net/sctp/auth.h
+++ b/include/net/sctp/auth.h
@@ -22,6 +22,7 @@ struct sctp_endpoint;
 struct sctp_association;
 struct sctp_authkey;
 struct sctp_hmacalgo;
+struct sctp_cookie;
 
 /* Defines an HMAC algorithm supported by SCTP chunk authentication */
 struct sctp_hmac {
@@ -72,6 +73,8 @@ struct sctp_shared_key *sctp_auth_get_shkey(
 int sctp_auth_asoc_copy_shkeys(const struct sctp_endpoint *ep,
 				struct sctp_association *asoc,
 				gfp_t gfp);
+bool sctp_auth_verify_cookie_params(const struct sctp_endpoint *ep,
+				    const struct sctp_cookie *cookie);
 const struct sctp_hmac *sctp_auth_get_hmac(__u16 hmac_id);
 const struct sctp_hmac *
 sctp_auth_asoc_get_hmac(const struct sctp_association *asoc);
diff --git a/net/sctp/auth.c b/net/sctp/auth.c
index c901d373af80..6162eb731a6c 100644
--- a/net/sctp/auth.c
+++ b/net/sctp/auth.c
@@ -377,6 +377,84 @@ int sctp_auth_asoc_copy_shkeys(const struct sctp_endpoint *ep,
 	return -ENOMEM;
 }
 
+static bool sctp_auth_cookie_chunk_forbidden(__u8 chunk_id)
+{
+	switch (chunk_id) {
+	case SCTP_CID_INIT:
+	case SCTP_CID_INIT_ACK:
+	case SCTP_CID_SHUTDOWN_COMPLETE:
+	case SCTP_CID_AUTH:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/* Verify AUTH parameters copied from a state cookie before they are restored
+ * into an association.  When cookie authentication is disabled these fields
+ * are peer-controlled, so they must satisfy the same constraints as locally
+ * generated AUTH parameters.
+ */
+bool sctp_auth_verify_cookie_params(const struct sctp_endpoint *ep,
+				    const struct sctp_cookie *cookie)
+{
+	const struct sctp_paramhdr *random;
+	const struct sctp_hmac_algo_param *hmacs;
+	const struct sctp_chunks_param *chunks;
+	bool has_sha1 = false;
+	__u16 hmacs_len;
+	__u16 chunks_len;
+	__u16 n_hmacs;
+	__u16 n_chunks;
+	__u16 i;
+
+	if (sctp_sk(ep->base.sk)->cookie_auth_enable || !ep->auth_enable)
+		return true;
+
+	random = (const struct sctp_paramhdr *)cookie->auth_random;
+	if (random->type != SCTP_PARAM_RANDOM ||
+	    ntohs(random->length) != sizeof(*random) + SCTP_AUTH_RANDOM_LENGTH)
+		return false;
+
+	hmacs = (const struct sctp_hmac_algo_param *)cookie->auth_hmacs;
+	hmacs_len = ntohs(hmacs->param_hdr.length);
+	if (hmacs->param_hdr.type != SCTP_PARAM_HMAC_ALGO ||
+	    hmacs_len < sizeof(struct sctp_paramhdr) +
+			sizeof(hmacs->hmac_ids[0]) ||
+	    hmacs_len > sizeof(cookie->auth_hmacs) ||
+	    (hmacs_len - sizeof(struct sctp_paramhdr)) %
+			sizeof(hmacs->hmac_ids[0]))
+		return false;
+
+	n_hmacs = (hmacs_len - sizeof(struct sctp_paramhdr)) /
+		  sizeof(hmacs->hmac_ids[0]);
+	for (i = 0; i < n_hmacs; i++) {
+		__u16 hmac_id = ntohs(hmacs->hmac_ids[i]);
+
+		if (!sctp_hmac_supported(hmac_id))
+			return false;
+		if (hmac_id == SCTP_AUTH_HMAC_ID_SHA1)
+			has_sha1 = true;
+	}
+	if (!has_sha1)
+		return false;
+
+	chunks = (const struct sctp_chunks_param *)cookie->auth_chunks;
+	chunks_len = ntohs(chunks->param_hdr.length);
+	if (chunks->param_hdr.type != SCTP_PARAM_CHUNKS ||
+	    chunks_len < sizeof(struct sctp_paramhdr) ||
+	    chunks_len > sizeof(cookie->auth_chunks))
+		return false;
+
+	n_chunks = chunks_len - sizeof(struct sctp_paramhdr);
+	for (i = 0; i < n_chunks; i++) {
+		if (sctp_auth_cookie_chunk_forbidden(chunks->chunks[i]))
+			return false;
+	}
+
+	return true;
+}
+
 
 /* Public interface to create the association shared key.
  * See code above for the algorithm.
@@ -447,6 +525,9 @@ struct sctp_shared_key *sctp_auth_get_shkey(
 
 const struct sctp_hmac *sctp_auth_get_hmac(__u16 hmac_id)
 {
+	if (!sctp_hmac_supported(hmac_id))
+		return NULL;
+
 	return &sctp_hmac_list[hmac_id];
 }
 
@@ -508,6 +589,11 @@ int sctp_auth_asoc_verify_hmac_id(const struct sctp_association *asoc,
 		return 0;
 
 	hmacs = (struct sctp_hmac_algo_param *)asoc->c.auth_hmacs;
+	if (ntohs(hmacs->param_hdr.length) < sizeof(struct sctp_paramhdr) ||
+	    ntohs(hmacs->param_hdr.length) > sizeof(asoc->c.auth_hmacs) ||
+	    !sctp_hmac_supported(ntohs(hmac_id)))
+		return 0;
+
 	n_elt = (ntohs(hmacs->param_hdr.length) -
 		 sizeof(struct sctp_paramhdr)) >> 1;
 
@@ -629,6 +715,8 @@ void sctp_auth_calculate_hmac(const struct sctp_association *asoc,
 	 */
 	key_id = ntohs(auth->auth_hdr.shkey_id);
 	hmac_id = ntohs(auth->auth_hdr.hmac_id);
+	if (!sctp_hmac_supported(hmac_id))
+		return;
 
 	if (key_id == asoc->active_key_id)
 		asoc_key = asoc->asoc_shared_key;
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 0ae30c3c8913..c08a5753fb58 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1852,6 +1852,9 @@ struct sctp_association *sctp_unpack_cookie(
 	/* Set up our peer's port number.  */
 	retval->peer.port = ntohs(chunk->sctp_hdr->source);
 
+	if (!sctp_auth_verify_cookie_params(ep, bear_cookie))
+		goto malformed;
+
 	/* Populate the association from the cookie.  */
 	memcpy(&retval->c, bear_cookie, sizeof(*bear_cookie));
 
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 708fa07d5fff..82836e927be6 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4436,6 +4436,9 @@ 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.47.3