Skip to content

Backport 3.6: Fix off-by-one over-read in TLS 1.2 ClientHello and CertificateRequest - #10928

Open
nvxbug wants to merge 3 commits into
Mbed-TLS:mbedtls-3.6from
nvxbug:tls12-handshake-len-bounds-3.6
Open

Backport 3.6: Fix off-by-one over-read in TLS 1.2 ClientHello and CertificateRequest#10928
nvxbug wants to merge 3 commits into
Mbed-TLS:mbedtls-3.6from
nvxbug:tls12-handshake-len-bounds-3.6

Conversation

@nvxbug

@nvxbug nvxbug commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Description

Backport of #10833 to mbedtls-3.6.

The TLS 1.2 ClientHello and CertificateRequest parsers read a two-byte length field (ciphersuite list on the server side, certificate authorities on the client side) that the preceding length checks did not fully reserve, so a message truncated inside that field was read one byte past its end. The checks now use the available < needed pattern, each field is checked before it is read, and an odd-length signature-algorithms list is rejected.

Both commits cherry-picked from development; only the new test entries' depends_on lines needed converting to the 3.6 macro names. The injected truncated-ClientHello tests pass in a TLS 1.2-only build (test_suite_ssl.tls-defrag 30/30).

PR checklist

  • changelog provided
  • 3.6 backport this PR
  • 2.28 backport not required because: 2.28 is end of life
  • tests provided

nvxbug added 3 commits August 24, 2026 21:58
Signed-off-by: Naveed <naveed@bugqore.com>
ClientHello: narrow the session-id check to the session id itself, check
the cookie length byte before reading it, and let the ciphersuite-list
length check cover both transports.

CertificateRequest: make the signature-algorithms check also reserve the
two certificate-authorities length bytes instead of adding a separate
check, use the available < needed pattern throughout, and reject an
odd-length signature-algorithms list.

Add injected truncated-ClientHello test cases (TLS 1.2-only builds).

Signed-off-by: Naveed <naveed@bugqore.com>
Signed-off-by: Naveed <naveed@bugqore.com>

@gilles-peskine-arm gilles-peskine-arm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, component-tls priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most) labels Aug 24, 2026
@gilles-peskine-arm gilles-peskine-arm moved this from In Development to In Review in Non-roadmap pull requests Aug 24, 2026
@github-project-automation github-project-automation Bot moved this from In Review to Has Approval in Non-roadmap pull requests Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component-tls needs-review Every commit must be reviewed by at least two team members, priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most)

Projects

Status: Has Approval

Development

Successfully merging this pull request may close these issues.

3 participants