[codex] cap SAML SLO message inflation - #184
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a58bc2f2d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let compact = encoded.split_whitespace().collect::<String>(); | ||
| reject_oversized_base64(&compact, max_message_size)?; |
There was a problem hiding this comment.
Bound the raw SAML payload before compaction
In deployments relying on max_logout_message_size as the SLO input cap, this guard runs only after encoded.split_whitespace().collect::<String>(), so an oversized SAMLRequest/SAMLResponse is fully scanned and copied before being rejected; if the excess is mostly whitespace, the compacted value can also remain below the cap and bypass this encoded-length check entirely. Check the raw length first or compact with a bounded accumulator so the new limit actually bounds attacker-controlled request work.
Useful? React with 👍 / 👎.
Summary
413 SAML_LOGOUT_MESSAGE_TOO_LARGEfor oversized LogoutRequest/LogoutResponse messages.Root Cause
SAML SLO Redirect binding messages were decoded and inflated from attacker-controlled
SAMLRequest/SAMLResponsevalues before any effective size limit or signature enforcement. A small compressed payload could expand into a much larger XML document and consume CPU/memory before rejection.Validation
cargo fmt --all --checkcargo nextest run -p rustauth-saml --all-featurescargo nextest run -p rustauth-sso --all-featurescargo clippy --workspace --all-targets --all-features -- -D warningsNotes
This branch is based on
origin/mainso it does not include the localAGENTS.mdcommit/change.