tls: bound the CertificateRequest certificate-type count - #415
Open
tinic wants to merge 1 commit into
Open
Conversation
_nx_secure_tls_process_certificate_request() read the certificate-type
count out of the message before establishing that the message had a byte
in it.
The only guard is
if (length >= message_length)
which sits inside the NX_SECURE_TLS_TLS_1_3_ENABLED arm, above the else.
For TLS 1.2 nothing runs before
cert_types_length = packet_buffer[length];
with length still 0, so a zero-length CertificateRequest reads one byte
past the record buffer. The sanity test that follows compares
cert_types_length against message_length, which is after the read.
A server sends CertificateRequest and chooses its length, so this is
reachable by any peer a client connects to.
Found by a fuzz driver over the client handshake path and confirmed under
AddressSanitizer.
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
1 participant
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.
_nx_secure_tls_process_certificate_request() read the certificate-type count out of the message before establishing that the message had a byte in it.
The only guard is
which sits inside the NX_SECURE_TLS_TLS_1_3_ENABLED arm, above the else. For TLS 1.2 nothing runs before
with length still 0, so a zero-length CertificateRequest reads one byte past the record buffer. The sanity test that follows compares cert_types_length against message_length, which is after the read.
A server sends CertificateRequest and chooses its length, so this is reachable by any peer a client connects to.
Found by a fuzz driver over the client handshake path and confirmed under AddressSanitizer.