Re: [PATCH net v2 2/2] sctp: add INIT verification after cookie unpacking
From: Simon Horman <horms@kernel.org>
Date: 2026-06-22 16:52:47
Also in:
linux-sctp
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 hunk ↗ jump to hunk
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 hunk ↗ jump to hunk
@@ -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?