Bug #3774: Assert failed in TLS due to integer underflow - Suricata - Open Information Security Foundation

Actions

Copy link

Bug #3774

closed

Assert failed in TLS due to integer underflow

Added by Philippe Antoine almost 4 years ago. Updated over 3 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Philippe Antoine

Target version:

6.0.0

Affected Versions:

5.0.3

Effort:

Difficulty:

Label:

Needs backport

Description

Found by oss-fuzz :
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22564

Stack trace is :

    #8 0x51f4b0 in SSLv3ParseHandshakeType suricata/src/app-layer-ssl.c:1666:9
    #9 0x51e780 in SSLv3ParseHandshakeProtocol suricata/src/app-layer-ssl.c:1736:18
    #10 0x51cef6 in SSLv3Decode suricata/src/app-layer-ssl.c:2421:26
    #11 0x51b6b7 in SSLDecode suricata/src/app-layer-ssl.c:2608:26
    #12 0x51a9d1 in SSLParseServerRecord suricata/src/app-layer-ssl.c:2658:12
    #13 0x50da1a in AppLayerParserParse suricata/src/app-layer-parser.c:1228:30
    #14 0x4c3860 in LLVMFuzzerTestOneInput suricata/src/tests/fuzz/fuzz_applayerparserparse.c:146:16

Files

clusterfuzz-testcase-minimized-fuzz_applayerparserparse-5111608992071680 (49 Bytes) clusterfuzz-testcase-minimized-fuzz_applayerparserparse-5111608992071680

Philippe Antoine, 06/16/2020 11:19 AM

Related issues 2 (0 open — 2 closed)

Actions

Copy link

Updated by Philippe Antoine almost 4 years ago

Bug analysis :
In SSLv3ParseHandshakeType, we use the condition

if (ssl_state->curr_connp->message_start + ssl_state->curr_connp->message_length <
            ssl_state->curr_connp->bytes_processed + input_len)

before assigning

write_len = (ssl_state->curr_connp->message_start + ssl_state->curr_connp->message_length) -
            ssl_state->curr_connp->bytes_processed;

But with this case, we end up having

ssl_state->curr_connp->message_length = 9
ssl_state->curr_connp->bytes_processed = 10
Leading to bad write_len = 0xFFFFFFFF

We seem to need the 5 packets of the reproducer to get this :

First packet (from server to client) has 9 bytes payload
After this packet, we have

ssl_state->curr_connp->message_length = 9
ssl_state->curr_connp->bytes_processed = 9

Second packet is from client to server to progress handshake somehow

Third packet is from server
It does not reach SSLv3ParseHandshakeType because of SSLv3Decode lines get into the break case :

    if (ssl_state->flags & SSL_AL_FLAG_CHANGE_CIPHER_SPEC) {
                /* In TLSv1.3, ChangeCipherSpec is only used for middlebox
                   compability (rfc8446, appendix D.4). */
                if ((ssl_state->client_connp.version > TLS_VERSION_12) &&
                       ((ssl_state->flags & SSL_AL_FLAG_STATE_SERVER_HELLO) == 0)) {
                    /* do nothing */
                } else {
                    break;
                }
            }

After this packet, we get

ssl_state->curr_connp->message_length = 9
ssl_state->curr_connp->bytes_processed = 10

Fourth packet is from the client and modifies ssl_state->client_connp.version so as to ssl_state->client_connp.version > TLS_VERSION_12 becomes true

Fifth and last packet from server triggers the integer underflow

Actions

Copy link