From e641c6b738d27c52741d5dcbc457ee33cb68ade7 Mon Sep 17 00:00:00 2001 From: John Bland Date: Wed, 27 Dec 2023 16:06:40 -0500 Subject: [PATCH 1/4] when removing the padding for the TLS13 verify message step, check that the index doesn't wrap around due to a malformed packet --- src/internal.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index 2274ab7e26..98dd6cda1e 100644 --- a/src/internal.c +++ b/src/internal.c @@ -21166,7 +21166,8 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) word16 i = (word16)(ssl->buffers.inputBuffer.idx + ssl->curSize - ssl->specs.aead_mac_size); - if (i > ssl->buffers.inputBuffer.length) { + /* check i isn't too big and won't wrap around on --i */ + if (i > ssl->buffers.inputBuffer.length || i == 0) { WOLFSSL_ERROR(BUFFER_ERROR); return BUFFER_ERROR; } From e1435e96d22d973817626f7d8f5d7debe6bb426c Mon Sep 17 00:00:00 2001 From: John Bland Date: Wed, 3 Jan 2024 17:21:08 -0500 Subject: [PATCH 2/4] do bounds check on full word32 size to match inputBuffer length --- src/internal.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index 98dd6cda1e..8189e3eaad 100644 --- a/src/internal.c +++ b/src/internal.c @@ -21162,16 +21162,19 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) ssl->keys.decryptedCur = 1; #ifdef WOLFSSL_TLS13 if (ssl->options.tls1_3) { - /* end of plaintext */ - word16 i = (word16)(ssl->buffers.inputBuffer.idx + - ssl->curSize - ssl->specs.aead_mac_size); - - /* check i isn't too big and won't wrap around on --i */ - if (i > ssl->buffers.inputBuffer.length || i == 0) { + /* check that the end of the logical length doesn't extend + * past the real buffer */ + word32 boundsCheck = (ssl->buffers.inputBuffer.idx + + ssl->curSize - ssl->specs.aead_mac_size); + if (boundsCheck > ssl->buffers.inputBuffer.length || + boundsCheck == 0) { WOLFSSL_ERROR(BUFFER_ERROR); return BUFFER_ERROR; } + /* end of plaintext */ + word16 i = (word16)(boundsCheck); + /* Remove padding from end of plain text. */ for (--i; i > ssl->buffers.inputBuffer.idx; i--) { if (ssl->buffers.inputBuffer.buffer[i] != 0) From 245c87fe8f6f16c3d51604f26bf2762c32023d0a Mon Sep 17 00:00:00 2001 From: John Bland Date: Wed, 3 Jan 2024 17:39:20 -0500 Subject: [PATCH 3/4] clean up variable definitions --- src/internal.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8189e3eaad..5033744b74 100644 --- a/src/internal.c +++ b/src/internal.c @@ -21162,10 +21162,11 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) ssl->keys.decryptedCur = 1; #ifdef WOLFSSL_TLS13 if (ssl->options.tls1_3) { - /* check that the end of the logical length doesn't extend - * past the real buffer */ + word16 i; word32 boundsCheck = (ssl->buffers.inputBuffer.idx + ssl->curSize - ssl->specs.aead_mac_size); + /* check that the end of the logical length doesn't extend + * past the real buffer */ if (boundsCheck > ssl->buffers.inputBuffer.length || boundsCheck == 0) { WOLFSSL_ERROR(BUFFER_ERROR); @@ -21173,7 +21174,7 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) } /* end of plaintext */ - word16 i = (word16)(boundsCheck); + i = (word16)(boundsCheck); /* Remove padding from end of plain text. */ for (--i; i > ssl->buffers.inputBuffer.idx; i--) { From b37716f5ce40f255d5e51cd63232521f8c144d54 Mon Sep 17 00:00:00 2001 From: John Bland Date: Wed, 3 Jan 2024 19:19:13 -0500 Subject: [PATCH 4/4] refactor and remove word16 index --- src/internal.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5033744b74..112c34b00f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -21162,20 +21162,15 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) ssl->keys.decryptedCur = 1; #ifdef WOLFSSL_TLS13 if (ssl->options.tls1_3) { - word16 i; - word32 boundsCheck = (ssl->buffers.inputBuffer.idx + + word32 i = (ssl->buffers.inputBuffer.idx + ssl->curSize - ssl->specs.aead_mac_size); /* check that the end of the logical length doesn't extend * past the real buffer */ - if (boundsCheck > ssl->buffers.inputBuffer.length || - boundsCheck == 0) { + if (i > ssl->buffers.inputBuffer.length || i == 0) { WOLFSSL_ERROR(BUFFER_ERROR); return BUFFER_ERROR; } - /* end of plaintext */ - i = (word16)(boundsCheck); - /* Remove padding from end of plain text. */ for (--i; i > ssl->buffers.inputBuffer.idx; i--) { if (ssl->buffers.inputBuffer.buffer[i] != 0)