[Bug]: Asan reports SEGV on unknown address in ProcessReplyEx, potential heap over flow

[hey3e]

Contact Details

heyme0920@gmail.com

Version

5.6.3 928dd70

Description

TLS1.3 client sends the Certificate packet whose record only contains authentication data, without cert and record type.

Build with asan.

Reproduction steps

1. export CFLAGS=-fsanitize=address
2. ./configure --enable-tls13
3. Use attached tls13.c to replace src/tls13.c, which modified BuildTls13Message at line 3193 and 3222, removing cert data and record type from record before encryption and hash.
   tls13.zip
4. make
5. ./examples/server/server -v 4 -i -x
6. ./examples/client/client -v 4
7. Asan reports SEGV on unknown address, if after many connections, asan may reports heap over flow.

Relevant log output

AddressSanitizer:DEADLYSIGNAL

=================================================================

==453426==ERROR: AddressSanitizer: SEGV on unknown address 0x60200001022f (pc 0x7fa94b172d95 bp 0x61a000000680 sp 0x7ffe0768ab00 T0)

==453426==The signal is caused by a READ memory access.

    #0 0x7fa94b172d95 in ProcessReplyEx (/home/gamin/Documents/wolfssl/src/.libs/libwolfssl.so.40+0xe3d95)

    #1 0x7fa94b1b4337 in wolfSSL_accept_TLSv13 (/home/gamin/Documents/wolfssl/src/.libs/libwolfssl.so.40+0x125337)

    #2 0x5652b5abe162 in server_test (/home/gamin/Documents/wolfssl/examples/server/.libs/server+0xd162)

    #3 0x5652b5aba18c in main (/home/gamin/Documents/wolfssl/examples/server/.libs/server+0x918c)

    #4 0x7fa94ae29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

    #5 0x7fa94ae29e3f in __libc_start_main_impl ../csu/libc-start.c:392

    #6 0x5652b5aba524 in _start (/home/gamin/Documents/wolfssl/examples/server/.libs/server+0x9524)



AddressSanitizer can not provide additional info.

SUMMARY: AddressSanitizer: SEGV (/home/gamin/Documents/wolfssl/src/.libs/libwolfssl.so.40+0xe3d95) in ProcessReplyEx

==453426==ABORTING

[anhu]

Hello @hey3e

Can you reproduce this problem without modifying wolfssl? Can you let us know why you modified the code?

[hey3e]

Sorry, I didn't explain clearly. We need to compile two wolfssl, one as server without modification, one as malicious client with modification.

Reproduction steps

Server

1. git checkout -f 928dd7021
2. export CFLAGS=-fsanitize=address
3. ./configure --enable-tls13
4. make
5. ./examples/server/server -v 4 -i -x

Malicious client

1. git checkout -f 928dd7021
2. Replace src/tls13.c
3. ./configure --enable-tls13
4. make
5. ./examples/client/client -v 4

In this case, technically we reproduced the problem without modifying wolfssl because the client is malicious in our attack model. Also the bug can be triggered without modifying client but it would be consuming because of encryption. I chose to leverage the client itself who originally can generate valid cryptographic packets. What I modify is just removing the cert data and record type of Certificate packet before its encryption.

[embhorn]

Here are the relevant changes from the zip:

diff --git a/src/tls13.c b/src/tls13.c
index 2386704c7..5a0a37330 100644
--- a/src/tls13.c
+++ b/src/tls13.c
@@ -3190,8 +3190,8 @@ int BuildTls13Message(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
         if (ssl->options.dtls)
             args->headerSz = Dtls13GetRlHeaderLength(ssl, 1);
 #endif /* WOLFSSL_DTLS13 */
-
-        args->sz = args->headerSz + inSz;
+        if (inSz == 1326) args->sz = args->headerSz + 0;
+        else args->sz = args->headerSz + inSz;
         args->idx  = args->headerSz;

     #ifdef WOLFSSL_ASYNC_CRYPT
@@ -3216,7 +3216,9 @@ int BuildTls13Message(WOLFSSL* ssl, byte* output, int outSz, const byte* input,
             }

             /* Record layer content type at the end of record data. */
-            args->sz++;
+
+            if (args->sz != 5) args->sz++;
+
             /* Authentication data at the end. */
             args->sz += ssl->specs.aead_mac_size;

[embhorn]

I was able to reproduce. I will request feedback from our engineers.

[jpbland1]

Hey @hey3e,

It seems we failed to do a bounds check during the mac verify step and the bad sizes that came in as input were causing our index variable to be 0, which then wrapped around to 0xffff in the next step after it was decremented. Anyways I've added a bounds check to return an error when this happens, here is the PR #7099. We'll close this issue once the PR has been merged, thank you for bringing this to our attention.

Best Wishes,
John Bland