In SCTP handshake, the INIT chunk is initially processed by the server and embedded into the cookie carried in INIT-ACK. The client then returns this cookie via COOKIE-ECHO, where the server unpacks it and reconstructs the original INIT chunk. When cookie authentication is enabled, the cookie contents are protected against tampering, so reusing the unpacked INIT without re-verification is safe. However, when cookie authentication is disabled, the reconstructed INIT can no longer be trusted. In this case, the INIT must be explicitly validated after unpacking to avoid processing potentially tampered data. Add sctp_verify_init() checks after cookie unpacking in COOKIE-ECHO processing paths (sctp_sf_do_5_1D_ce() and sctp_sf_do_5_2_4_dupcook()) when cookie_auth_enable is disabled. On failure, the association is freed and an ABORT is generated via sctp_abort_on_init_err(). Also update sctp_verify_init() to validate parameter bounds against the actual peer_init length rather than chunk->chunk_end, since peer_init may not span the full chunk buffer in COOKIE-ECHO. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Xin Long --- v2: - Because of sctp_abort_on_init_err() param change in patch 1/2, pass cid and not chunk. - Use SCTP_PAD4() around ntohs(peer_init->chunk_hdr.length) when checking param.v in sctp_verify_init() to make Sashiko happy. --- net/sctp/sm_make_chunk.c | 3 ++- net/sctp/sm_statefuns.c | 33 ++++++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 41958b8e59fd..d5ee81934d93 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -2298,7 +2298,8 @@ int sctp_verify_init(struct net *net, const struct sctp_endpoint *ep, * VIOLATION error. We build the ERROR chunk here and let the normal * error handling code build and send the packet. */ - if (param.v != (void *)chunk->chunk_end) + if (param.v != (void *)peer_init + + SCTP_PAD4(ntohs(peer_init->chunk_hdr.length))) return sctp_process_inv_paramlength(asoc, param.p, chunk, errp); /* The only missing mandatory param possible today is 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); + } + if (err_chk_p) + sctp_chunk_free(err_chk_p); + if (security_sctp_assoc_request(new_asoc, chunk->head_skb ?: chunk->skb)) { sctp_association_free(new_asoc); return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); @@ -741,7 +754,6 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net, /* This is a brand-new association, so these are not yet side * effects--it is safe to run them here. */ - peer_init = (struct sctp_init_chunk *)(chunk->subh.cookie_hdr + 1); if (!sctp_process_init(new_asoc, chunk, &chunk->subh.cookie_hdr->c.peer_addr, peer_init, GFP_ATOMIC)) @@ -2133,10 +2145,12 @@ enum sctp_disposition sctp_sf_do_5_2_4_dupcook( void *arg, struct sctp_cmd_seq *commands) { + struct sctp_chunk *err_chk_p = NULL; struct sctp_association *new_asoc; + struct sctp_init_chunk *peer_init; struct sctp_chunk *chunk = arg; enum sctp_disposition retval; - struct sctp_chunk *err_chk_p; + enum sctp_cid cid; int error = 0; char action; @@ -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; case 'D': /* Collision case D. */ /* Update socket peer label if first association. */ if (security_sctp_assoc_request((struct sctp_association *)asoc, -- 2.47.1