2027370 - Heap buffer overflow via intra-struct OOB write in [@ ssl3_HandleSigAlgsXtn] from repeated TLS 1.3 post-handshake CertificateRequest

Status: RESOLVED DUPLICATE of bug 2026089

(NSS :: Libraries, defect)

Description

[Bugmon [:jkratzer for issues]]

Summary

An out-of-bounds write in the fixed-size TLSExtensionData.negotiated[] array (security/nss/lib/ssl/ssl3exthandle.c:1642) allows a malicious TLS 1.3 server to corrupt adjacent pointer fields in the TLSExtensionData struct by sending repeated post-handshake CertificateRequest messages. The numNegotiated counter is never reset between post-handshake messages, so each CertificateRequest increments it without bound. After enough iterations, writes corrupt the sigSchemes pointer, which is then dereferenced in PORT_Memcpy at ssl3con.c:8271, causing a heap-buffer-overflow with a partially attacker-influenced pointer.

Affected Code

File: security/nss/lib/ssl/ssl3exthandle.c, line 1642

SECStatus
ssl3_HandleSigAlgsXtn(const sslSocket *ss, TLSExtensionData *xtnData,
                      SECItem *data)
{
    ...
    /* Keep track of negotiated extensions. */
    xtnData->negotiated[xtnData->numNegotiated++] = ssl_signature_algorithms_xtn;
    return SECSuccess;
}

File: security/nss/lib/ssl/tls13exthandle.c, line 2013 (same pattern, also in CertificateRequest handler table)

SECStatus
ssl3_HandleCertificateCompressionXtn(const sslSocket *ss,
                                     TLSExtensionData *xtnData, SECItem *data)
{
    ...
    xtnData->negotiated[xtnData->numNegotiated++] = ssl_certificate_compression_xtn;
    ...
}

File: security/nss/lib/ssl/ssl3ext.h, lines 44-45

struct TLSExtensionDataStr {
    ...
    PRUint16 numNegotiated;
    PRUint16 negotiated[SSL_MAX_EXTENSIONS];  /* SSL_MAX_EXTENSIONS = 22 */
    /* Fields below are corrupted by OOB writes: */
    PRBool ticketTimestampVerified;
    ...
    SECItem psk_ke_modes;
    ...
    SECItem *sniNameArr;
    ...
    SSLSignatureScheme *sigSchemes;   /* <-- corrupted, then dereferenced */
    unsigned int numSigSchemes;
    ...
};

File: security/nss/lib/ssl/tls13con.c, lines 3283-3353

static SECStatus
tls13_HandleCertificateRequest(sslSocket *ss, PRUint8 *b, PRUint32 length)
{
    ...
    if (tls13_IsPostHandshake(ss)) {
        /* clean up anything left from previous handshake. */
        /* NOTE: numNegotiated is NOT reset here */
        ...
        SECITEM_FreeItem(&ss->xtnData.certReqContext, PR_FALSE);
        ss->xtnData.certReqContext.data = NULL;
    }
    ...
    /* Process all the extensions. */
    rv = ssl3_HandleExtensions(ss, &extensionsData.data, &extensionsData.len,
                               ssl_hs_certificate_request);
    /* ^^^ increments numNegotiated via ssl3_HandleSigAlgsXtn */

The negotiated[] array is sized for a single handshake (SSL_MAX_EXTENSIONS = 22), and numNegotiated is only reset by ssl3_ResetExtensionData, which is called at handshake start but not between post-handshake CertificateRequest messages. RFC 8446 permits servers to send multiple post-handshake CertificateRequest messages over a connection's lifetime. Each one triggers ssl3_HandleSigAlgsXtn (the signature_algorithms extension is mandatory in CertificateRequest), incrementing numNegotiated past 22 and writing 0x000d into adjacent struct fields. Because this is an intra-struct overflow, ASAN does not catch the write directly; it catches the subsequent use of the corrupted sigSchemes pointer.

The same unbounded negotiated[numNegotiated++] pattern appears in 12 locations in tls13exthandle.c (lines 459, 759, 829, 873, 898, 1119, 1156, 1227, 1572, 1637, 1727, 2013) and numerous locations in ssl3exthandle.c, none with bounds checking.

Exploit Chain

1. Victim (NSS client) enables SSL_ENABLE_POST_HANDSHAKE_AUTH and connects to an attacker-controlled TLS 1.3 server.
2. Normal TLS 1.3 handshake completes; numNegotiated holds the count of extensions negotiated during the initial handshake (typically 5-10).
3. Server sends post-handshake CertificateRequest #1 containing signature_algorithms extension → ssl3_HandleSigAlgsXtn writes negotiated[numNegotiated++] = 0x000d.
4. Client responds with Certificate/CertificateVerify/Finished.
5. Server repeats step 3. After ~15-20 iterations, writes exceed negotiated[21] and begin corrupting ticketTimestampVerified, psk_ke_modes, sniNameArr, signedCertTimestamps.
6. After ~40-60 iterations, writes reach the sigSchemes pointer field, overwriting its low bytes with 0x000d000d... pattern.
7. On the next CertificateRequest, tls13_HandleCertificateRequest calls ssl3_BeginHandleCertificateRequest(ss, ss->xtnData.sigSchemes, ss->xtnData.numSigSchemes, ...).
8. PORT_Memcpy at ssl3con.c:8271 reads from the corrupted sigSchemes pointer (0x...000d) → heap-buffer-overflow.

Security Impact

Severity: High (sec-high candidate)

Attacker capability: A malicious TLS 1.3 server can achieve controlled intra-struct memory corruption on a connected NSS client. The written value is the extension ID (attacker can influence which extensions are sent in CertificateRequest, choosing from signature_algorithms = 0x000d, signature_algorithms_cert = 0x0032, certificate_authorities = 0x002f, status_request = 0x0005, signed_certificate_timestamp = 0x0012). By varying the extension set across iterations, the attacker controls which 16-bit values are sprayed into the struct. Corruptible fields include multiple heap pointers (sigSchemes, sniNameArr, psk_ke_modes.data, delegCredSigSchemes) that are later passed to PORT_Free in ssl3_DestroyExtensionData or dereferenced in PORT_Memcpy. While full pointer control is limited (only low bytes are overwritten with small values), this provides a primitive for partial pointer overwrite that could be chained with heap grooming for arbitrary-free or wild-copy.

Preconditions:

• Client must set SSL_ENABLE_POST_HANDSHAKE_AUTH = PR_TRUE (opt-in feature, not default).
• Client must connect to attacker-controlled server over TLS 1.3.
• Client must respond to each CertificateRequest (requires a getClientAuthData callback).

Suggested Fix

Primary fix in tls13con.c:tls13_HandleCertificateRequest:

if (tls13_IsPostHandshake(ss)) {
    ...
    SECITEM_FreeItem(&ss->xtnData.certReqContext, PR_FALSE);
    ss->xtnData.certReqContext.data = NULL;
+   /* Reset extension tracking for this post-handshake exchange to
+    * prevent unbounded growth of numNegotiated. */
+   ss->xtnData.numNegotiated = 0;
+   if (ss->xtnData.sigSchemes) {
+       PORT_Free(ss->xtnData.sigSchemes);
+       ss->xtnData.sigSchemes = NULL;
+       ss->xtnData.numSigSchemes = 0;
+   }
}

Defense-in-depth: add a bounds-checked helper in ssl3ext.c and replace all 30+ open-coded negotiated[numNegotiated++] sites across tls13exthandle.c and ssl3exthandle.c:

void
ssl3_RecordNegotiatedExtension(TLSExtensionData *xtnData, PRUint16 ex_type)
{
    if (xtnData->numNegotiated >= SSL_MAX_EXTENSIONS) {
        PORT_Assert(0); /* array too small */
        return;
    }
    xtnData->negotiated[xtnData->numNegotiated++] = ex_type;
}

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: ssl_auth_unittest.cc

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: crash_stack.txt

Comment 5

[Dennis Jackson]

This is a duplicate of bug 2026089 and also 2027326 filed in the same batch. FWIW the report quality of this bug is much higher than the original!

Comment 6

[Bugmon [:jkratzer for issues]]

No valid actions for resolution (DUPLICATE).
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.