On Mon, Jun 22, 2026 at 12:52 PM Simon Horman [off-list ref] wrote:

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://netdev-ai.bots.linux.dev/sashiko/
---
sctp: add INIT verification after cookie unpacking

This commit adds sctp_verify_init() checks in the COOKIE-ECHO processing
paths sctp_sf_do_5_1D_ce() and sctp_sf_do_5_2_4_dupcook() when
cookie_auth_enable is disabled, and updates sctp_verify_init() to bound
the parameter walk against peer_init length rather than chunk->chunk_end.
On verification failure the new association is freed and an ABORT is
generated via sctp_abort_on_init_err().

quoted

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 8c636f045e45..6967e889d1bd 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c

@@ -650,11 +650,12 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
                                       struct sctp_cmd_seq *commands)
 {
      struct sctp_ulpevent *ev, *ai_ev = NULL, *auth_ev = NULL;
+     struct sctp_chunk *err_chk_p = NULL;
      struct sctp_association *new_asoc;
      struct sctp_init_chunk *peer_init;
      struct sctp_chunk *chunk = arg;
-     struct sctp_chunk *err_chk_p;
      struct sctp_chunk *repl;
+     enum sctp_cid cid;
      struct sock *sk;
      int error = 0;

@@ -728,6 +729,18 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
              }
      }

+     peer_init = (struct sctp_init_chunk *)(chunk->subh.cookie_hdr + 1);
+     cid = peer_init->chunk_hdr.type;
+     if (!sctp_sk(sk)->cookie_auth_enable &&
+         !sctp_verify_init(net, ep, asoc, cid, peer_init, chunk,
+                           &err_chk_p)) {
+             sctp_association_free(new_asoc);
+             return sctp_abort_on_init_err(net, ep, asoc, cid, arg, commands,
+                                           err_chk_p);
+     }

[Critical]
When cookie_auth_enable is disabled the cookie HMAC is not validated by
sctp_unpack_cookie(), so every byte of the embedded INIT (including
peer_init->chunk_hdr.type) is attacker-controlled.  Is it safe to feed
that byte directly into both sctp_verify_init() and
sctp_abort_on_init_err() as cid?

Inside the COOKIE-ECHO body the embedded chunk is by RFC always an INIT.
Could the cid argument be hard-coded to SCTP_CID_INIT (or
peer_init->chunk_hdr.type be validated against SCTP_CID_INIT before use)
instead of trusting the wire byte?

Two attacker-reachable consequences look possible if cid is forged to
SCTP_CID_INIT_ACK (3) and the STATE_COOKIE parameter is omitted:

sctp_verify_init() then returns 0 via this branch with *errp populated:

        if ((SCTP_CID_INIT_ACK == cid) && !has_cookie)
                return sctp_process_missing_param(asoc, SCTP_PARAM_STATE_COOKIE,
                                                  chunk, errp);

control transfers to sctp_abort_on_init_err(net, ep, asoc, cid, ...) with
cid == SCTP_CID_INIT_ACK and a non-NULL err_chunk.  After sending the
ABORT packet, that helper falls through to its out: label because
cid == SCTP_CID_INIT_ACK:

        if (cid != SCTP_CID_INIT_ACK) {
                if (!packet)
                        return SCTP_DISPOSITION_NOMEM;
                return SCTP_DISPOSITION_CONSUME;
        }

out:
        ...
        return sctp_stop_t1_and_abort(net, commands, error, ECONNREFUSED,
                                      asoc, chunk->transport);

In the sctp_sf_do_5_1D_ce() context, asoc is typically NULL (the function
itself opens with if (asoc && !sctp_vtag_verify(chunk, asoc))).  Can the
SCTP_CMD_NEW_STATE / SCTP_CMD_TIMER_STOP / SCTP_CMD_INIT_FAILED commands
queued by sctp_stop_t1_and_abort() against a NULL asoc be dispatched
without the sideeffect interpreter dereferencing asoc?

quoted

@@ -2205,6 +2219,19 @@ enum sctp_disposition sctp_sf_do_5_2_4_dupcook(
      switch (action) {
      case 'A': /* Association restart. */
      case 'B': /* Collision case B. */
+             peer_init = (struct sctp_init_chunk *)
+                             (chunk->subh.cookie_hdr + 1);
+             cid = peer_init->chunk_hdr.type;
+             if (!sctp_sk(ep->base.sk)->cookie_auth_enable &&
+                 !sctp_verify_init(net, ep, asoc, cid, peer_init, chunk,
+                                   &err_chk_p)) {
+                     sctp_association_free(new_asoc);
+                     return sctp_abort_on_init_err(net, ep, asoc, cid, arg,
+                                                   commands, err_chk_p);
+             }
+             if (err_chk_p)
+                     sctp_chunk_free(err_chk_p);
+             fallthrough;

[Critical]
The same attacker-controlled cid is propagated here as well.  In
sctp_sf_do_5_2_4_dupcook() asoc is the existing established association.
With a forged cid of SCTP_CID_INIT_ACK and missing STATE_COOKIE, can a
single unauthenticated COOKIE-ECHO drive sctp_abort_on_init_err() into
its out: path and have sctp_stop_t1_and_abort() tear down the established
asoc and report ECONNREFUSED to the application?

Would constraining the verification to the well-defined INIT case (for
example, hard-coding SCTP_CID_INIT or rejecting any cid !=
SCTP_CID_INIT before calling sctp_verify_init()/sctp_abort_on_init_err())
avoid both of these paths?

I will place a check in sctp_unpack_cookie() for such malformed packets:

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index d5ee81934d93..8adac9e0cd66 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c

@@ -1761,6 +1761,8 @@ struct sctp_association *sctp_unpack_cookie(
        bear_cookie = &cookie->c;

        ch = (struct sctp_chunkhdr *)(bear_cookie + 1);
+       if (ch->type != SCTP_CID_INIT)
+               goto malformed;
        chlen = ntohs(ch->length);
        if (chlen < sizeof(struct sctp_init_chunk))
                goto malformed;

Thanks.