Skip to content

Fix false positives in WOLFSSL_CIPHER_TEXT_CHECK for small records#10288

Closed
Frauschi wants to merge 1 commit intowolfSSL:masterfrom
Frauschi:test_fix
Closed

Fix false positives in WOLFSSL_CIPHER_TEXT_CHECK for small records#10288
Frauschi wants to merge 1 commit intowolfSSL:masterfrom
Frauschi:test_fix

Conversation

@Frauschi
Copy link
Copy Markdown
Contributor

Problem

The glitch-detection check guarded by WOLFSSL_CIPHER_TEXT_CHECK (enabled by --enable-maxstrength) saves up to 8 bytes of plaintext before encryption and compares them to the start of the ciphertext afterward. If they match, it assumes the AEAD pass-through'd and returns ENCRYPT_ERROR.

The copy and compare both used min(dataSz, sizeof(sanityCheck)), so when the plaintext is smaller than 8 bytes the check only compares that many bytes of ciphertext, which collides legitimately with random probability 1/256^dataSz. For a 1-byte plaintext (e.g. an empty TLS 1.3 application-data record, whose only plaintext is the content-type byte) that's ~1/256 per record.

This surfaced as sporadic failures of test_tls13_empty_record_limit introduced in #10187, which builds WOLFSSL_MAX_EMPTY_RECORDS such records in a row, resulting in a combined false-positive rate of ~8–9%.

Fix

In both EncryptTls13 (src/tls13.c) and Encrypt (src/internal.c), only arm the sanity check when there is a full sizeof(sanityCheck) (8) bytes of plaintext, and always compare the full 8 bytes. Shorter records skip the check entirely rather than running an unreliable version of it (a 1-byte glitch check isn't trustworthy anyway).

@Frauschi Frauschi mentioned this pull request Apr 23, 2026
@github-actions
Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

@Frauschi
Copy link
Copy Markdown
Contributor Author

Closing as the same fixes have been merged to master in #10296

@Frauschi Frauschi closed this Apr 24, 2026
@Frauschi Frauschi deleted the test_fix branch April 24, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants