Reject out-of-bounds ALPN length in ssl_context_load#10792
Open
nvxbug wants to merge 1 commit into
Open
Conversation
The serialized ALPN length was passed to memcmp() against each configured protocol without checking it against the remaining serialized data, so a malformed context could read past the end of the buffer. Bound it before the comparison, like the adjacent CID fields. Signed-off-by: Naveed <naveed@bugqore.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
ssl_context_load()reads the serialized ALPN protocol length as a single byte and then runsmemcmp(p, *cur, alpn_len)against each configured protocol, but it never checks thatalpn_lenbytes still remain beforeend. A corrupted or truncated serialized context whose ALPN length is larger than the bytes that actually follow makes thatmemcmpread past the end of the buffer.The added check bounds
alpn_lenagainst the remaining data right after the length is read, the same way the DTLS CID lengths a few lines above are bounded. Keeping the check next to the read leaves both the comparison loop and thep += alpn_lenadvance inside the buffer. The regression test undertest_suite_sslserializes a context with a negotiated protocol, enlarges the stored length, and checks the load is rejected; it trips ASan at thememcmpbefore the fix.PR checklist