Skip to content

Commit 29f674e

Browse files
avoid glitch hardening false positive byte collision with small messages and adjust test case
1 parent 72c7d12 commit 29f674e

3 files changed

Lines changed: 31 additions & 25 deletions

File tree

src/internal.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20682,9 +20682,10 @@ static WC_INLINE int Encrypt(WOLFSSL* ssl, byte* out, const byte* input,
2068220682
}
2068320683

2068420684
#ifdef WOLFSSL_CIPHER_TEXT_CHECK
20685-
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null) {
20685+
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null &&
20686+
sz >= sizeof(ssl->encrypt.sanityCheck)) {
2068620687
XMEMCPY(ssl->encrypt.sanityCheck, input,
20687-
min(sz, sizeof(ssl->encrypt.sanityCheck)));
20688+
sizeof(ssl->encrypt.sanityCheck));
2068820689
}
2068920690
#endif
2069020691

@@ -20771,8 +20772,9 @@ static WC_INLINE int Encrypt(WOLFSSL* ssl, byte* out, const byte* input,
2077120772
{
2077220773
#ifdef WOLFSSL_CIPHER_TEXT_CHECK
2077320774
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null &&
20775+
sz >= sizeof(ssl->encrypt.sanityCheck) &&
2077420776
XMEMCMP(out, ssl->encrypt.sanityCheck,
20775-
min(sz, sizeof(ssl->encrypt.sanityCheck))) == 0) {
20777+
sizeof(ssl->encrypt.sanityCheck)) == 0) {
2077620778

2077720779
WOLFSSL_MSG("Encrypt sanity check failed! Glitch?");
2077820780
WOLFSSL_ERROR_VERBOSE(ENCRYPT_ERROR);

src/tls13.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2639,9 +2639,9 @@ static int EncryptTls13(WOLFSSL* ssl, byte* output, const byte* input,
26392639

26402640
#ifdef WOLFSSL_CIPHER_TEXT_CHECK
26412641
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null &&
2642-
dataSz > 0) {
2642+
dataSz >= sizeof(ssl->encrypt.sanityCheck)) {
26432643
XMEMCPY(ssl->encrypt.sanityCheck, input,
2644-
min(dataSz, sizeof(ssl->encrypt.sanityCheck)));
2644+
sizeof(ssl->encrypt.sanityCheck));
26452645
}
26462646
#endif
26472647

@@ -2825,9 +2825,9 @@ static int EncryptTls13(WOLFSSL* ssl, byte* output, const byte* input,
28252825

28262826
#ifdef WOLFSSL_CIPHER_TEXT_CHECK
28272827
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null &&
2828-
dataSz > 0 &&
2828+
dataSz >= sizeof(ssl->encrypt.sanityCheck) &&
28292829
XMEMCMP(output, ssl->encrypt.sanityCheck,
2830-
min(dataSz, sizeof(ssl->encrypt.sanityCheck))) == 0) {
2830+
sizeof(ssl->encrypt.sanityCheck)) == 0) {
28312831

28322832
WOLFSSL_MSG("EncryptTls13 sanity check failed! Glitch?");
28332833
return ENCRYPT_ERROR;

tests/api/test_tls13.c

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4590,7 +4590,7 @@ int test_tls13_empty_record_limit(void)
45904590
struct test_memio_ctx test_ctx;
45914591
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
45924592
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
4593-
int recSz;
4593+
int recSz = 0;
45944594
/* Send exactly WOLFSSL_MAX_EMPTY_RECORDS to pin the boundary check.
45954595
* The Nth record increments the counter to N, and `N >= N` triggers
45964596
* the error. Sending one more would let a `>=` -> `>` mutation survive
@@ -4608,16 +4608,18 @@ int test_tls13_empty_record_limit(void)
46084608

46094609
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
46104610

4611-
/* Consume any post-handshake messages (e.g. NewSessionTicket). */
4612-
wolfSSL_read(ssl_c, buf, sizeof(buf));
4613-
test_memio_clear_buffer(&test_ctx, 0);
4614-
test_memio_clear_buffer(&test_ctx, 1);
4615-
4616-
/* Get the size of an encrypted zero-length app data record. */
4617-
recSz = BuildTls13Message(ssl_c, NULL, 0, NULL, 0,
4618-
application_data, 0, 1, 0);
4619-
ExpectIntGT(recSz, 0);
4620-
ExpectIntLE(recSz, (int)sizeof(rec));
4611+
if (EXPECT_SUCCESS()) {
4612+
/* Consume any post-handshake messages (e.g. NewSessionTicket). */
4613+
wolfSSL_read(ssl_c, buf, sizeof(buf));
4614+
test_memio_clear_buffer(&test_ctx, 0);
4615+
test_memio_clear_buffer(&test_ctx, 1);
4616+
4617+
/* Get the size of an encrypted zero-length app data record. */
4618+
recSz = BuildTls13Message(ssl_c, NULL, 0, NULL, 0,
4619+
application_data, 0, 1, 0);
4620+
ExpectIntGT(recSz, 0);
4621+
ExpectIntLE(recSz, (int)sizeof(rec));
4622+
}
46214623

46224624
/* Build all empty records into one contiguous buffer. */
46234625
if (EXPECT_SUCCESS()) {
@@ -4664,15 +4666,17 @@ int test_tls13_empty_record_limit(void)
46644666

46654667
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
46664668

4667-
wolfSSL_read(ssl_c, buf, sizeof(buf));
4668-
test_memio_clear_buffer(&test_ctx, 0);
4669-
test_memio_clear_buffer(&test_ctx, 1);
4669+
if (EXPECT_SUCCESS()) {
4670+
wolfSSL_read(ssl_c, buf, sizeof(buf));
4671+
test_memio_clear_buffer(&test_ctx, 0);
4672+
test_memio_clear_buffer(&test_ctx, 1);
46704673

4671-
recSz = BuildTls13Message(ssl_c, NULL, 0, NULL, 0,
4672-
application_data, 0, 1, 0);
4673-
ExpectIntGT(recSz, 0);
4674+
recSz = BuildTls13Message(ssl_c, NULL, 0, NULL, 0,
4675+
application_data, 0, 1, 0);
4676+
ExpectIntGT(recSz, 0);
4677+
}
46744678

4675-
{
4679+
if (EXPECT_SUCCESS()) {
46764680
int emptyBefore = WOLFSSL_MAX_EMPTY_RECORDS - 1;
46774681
int emptyAfter = WOLFSSL_MAX_EMPTY_RECORDS - 1;
46784682
int dataRecSz;

0 commit comments

Comments
 (0)