wolfSSL 5.3.0 Denial Of Service

# wolfSSL 5.3.0: Denial-of-service==================================## INFO=======The CVE project has assigned the id CVE-2022-38153 to this issue.Severity: 5.9 MEDIUMAffected version: 5.3.0End of embargo: Ended August 30, 2022Blog Post: https://blog.trailofbits.com/2023/01/12/wolfssl-vulnerabilities-tlspuffin-fuzzing-ssh/## SUMMARY==========In wolfSSL 5.3.0 man-in-the-middle attackers or a malicious server can crash TLS1.2 clients during a handshake. If an attacker injects a large ticket (above 256bytes) into a NewSessionTicket message in a TLS 1.2 handshake, and the clienthas a non-empty session cache, the session cache frees a pointer which points tonon-allocated memory, causing the client to crash with a “free(): invalidpointer”. Note: It is likely that this is also exploitable in TLS 1.3 handshakesbetween a client and a malicious server. With TLS 1.3 it is not possible toexploit this as a man-in-the-middle. This bug was discovered using the novelsymbolic-model-guided fuzzer tlspuffin.## DETAILS==========Line numbers below are valid for the wolfSSL Git tag v5.3.0-stable. The bugexists in the AddSessionToCache function in line ssh.c:13405. Thedenial-of-service bug is only exploitable if the --enable-session-ticket compileflag is used and the cache row of the session cache has an existing entry. Thisis a random process because the used cache row depends on the hash of thesession id. Approximately 30 cached sessions are required in order to reach thebug reliably. To exploit the bug, an attacker can inject a NewSessionTicketmessage into the handshake with a large ticket (for example 700 bytes). It mustbe larger than SESSION_TICKET_LEN in order to trigger a memory allocation inssl.c:13442.```    /* Alloc Memory here to avoid syscalls during lock */    if (ticLen > SESSION_TICKET_LEN) {        ticBuff = (byte*)XMALLOC(ticLen, NULL,                DYNAMIC_TYPE_SESSION_TICK);        …    }```Because of this allocation, the condition in line ssl.c:13500 is true, whichsets cacheTicBuff pointer to the ticket of some cached session.```    /* If we can re-use the existing buffer in cacheSession then we won't touch     * ticBuff at all making it a very cheap malloc/free. The page on a modern      OS will most likely not even be allocated to the process. /    if (ticBuff != NULL && cacheSession->ticketLenAlloc < ticLen) {        cacheTicBuff = cacheSession->ticket;        …    }```This ticket is not allocated but is equal to the _staticTicket buffer, ifprevious session’s tickets are smaller than SESSION_TICKET_LEN. This means thatcacheTicBuff points now to the _staticTicket buffer which is part of structWOLFSSL_SESSION. During the execution of wolfSSL_DupSession, the cached sessiongets overwritten by the new session.```    /* Copy data into the cache object */    ret = wolfSSL_DupSession(addSession, cacheSession, 1) == WOLFSSL_FAILURE;```A XMEMCPY in wolfSSL_DupSession copies fields from the new session to the cachedsession.```    XMEMCPY((byte*)output + copyOffset, (byte*)input + copyOffset,            sizeof(WOLFSSL_SESSION) - copyOffset);```Unfortunately, cacheTicBuff still points to _staticTicket after the copy. Inline ssl.c:13557 cacheTicBuff is freed which causes the process to crash.```    if (cacheTicBuff != NULL)        XFREE(cacheTicBuff, NULL, DYNAMIC_TYPE_SESSION_TICK);```## AFFECTED VERSIONS====================The bug does not exist in versions before wolfSSL 5.3.0. This bug is no longerexploitable on the current master branch with Git reference 218ab7e and version5.4.0. The bug is circumvented on clients by the if condition in linessl.c:13692 with Git commit b6b007d. It is unclear whether this change wasintended as it is unrelated to the commit message. It could potentially bereverted in the future if client-side session caching is wanted.If wolfSSL clients make calls to wolfSSL_get_session, then this bug is stillexploitable. The function wolfSSL_get_session also calls AddSessionToCache onclients. We observed that no other function calls AddSessionToCache.The vulnerable code is still reachable on wolfSSL 5.4.0 servers, but the bug isnot exploitable because TLS clients can not control the length of sessiontickets.## SUGGESTED REMEDIATION========================Short term, check if ticketLenAlloc is not zero before freeing cacheTicBuff.This ensures that the cacheTicBuff is only freed if the cached session containsan allocated ticket. Long term, simplify the session cache to preventcross-session logical validation. Furthermore, we recommend that the static andglobal mutex-protected session cache should be stored in the SSL context.Therefore, its lifetime would be bound to the lifetime of the SSL context. Bylimiting its lifetime the exploitability of caching related bugs is reduced.