1867408 - (CVE-2024-0743) Crash in [@ ssl_SendSavedWriteData | ssl3_SendRecord | ssl3_FlushHandshakeMessages | ssl3_FlushHandshake | tls13_SendClientSecondFlight ]

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[John Schanck [:jschanck]]

We see a few hundred instances per month of an EXC_BAD_ACCESS crash in ssl_SendSavedWriteData on macOS.

The crash is in this memmove:

        rv = ssl_DefSend(ss, ss->pendingBuf.buf, ss->pendingBuf.len, 0);
        if (rv < 0) {
            return rv;
        }
        ss->pendingBuf.len -= rv;
        if (ss->pendingBuf.len > 0 && rv > 0) {
            /* UGH !! This shifts the whole buffer down by copying it */
            PORT_Memmove(ss->pendingBuf.buf, ss->pendingBuf.buf + rv,
                         ss->pendingBuf.len);

        }

We're failing to check that ss->pendingBuf.len >= rv before performing ss->pendingBuf.len -= rv;. The ss->pendingBuf.len > 0 clause in the if statement won't catch this error because ss->pendingBuf.len is unsigned.

I'll post a patch to avoid the symptom, but the underlying problem (rv > ss->pendingBuf.len) should also be addressed. I don't know the code (or macOS) well enough to see why that would happen.

Comment 1

[John Schanck [:jschanck]]

Attachment: Bug 1867408 - add a defensive check for large ssl_DefSend return values. r=nkulatova

Comment 2

[Daniel Veditz [:dveditz]]

I only see 21 crashes the last 3 months for [@ ssl_SendSavedWriteData | ssl3_SendRecord | ssl3_FlushHandshake | tls13_SendClientSecondFlight], and they are all on the unsupported ESR-102 branch. The crashes stop in October so I imagine those folks got upgraded to ESR-115

For the other signature, ESR-115 releases have a lot more crashes than the mainline releases. If we throttle Mac release crashes as we do Windows that would make sense, but I was pretty sure the 10% sampling was for "Release on Windows" only. We have fewer users on other OSs and channels so we need to process all the crashes we get. Either I'm wrong about the sampling, or it looks like we've fixed at least one cause that lead to this crash signature since Firefox 115.

Guessing this could be sec-high because it's an out of bounds issue, assuming this is triggered by data sent from an attacker's site

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:jschanck, could you consider increasing the severity of this security bug?

For more information, please visit BugBot documentation.

Comment 4

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/1bda168c0da97e19e5f14bc4227c15c0a9f493bf

(NSS follows slightly different practices than Firefox for sec revisions. We approved the landing of this patch in our weekly meeting.)

Comment 5

[Jesse Schwartzentruber (:truber)]

Attachment: advisory.txt

Comment 6

[Giorgio Maone [:ma1]]

Is there a timeline for this fix to be available in firefox-esr115?

Comment 7

[John Schanck [:jschanck]]

This is included in NSS 3.90.2 (released on Feb 15, 2024). Uplift to ESR 115 is tracked in Bug 1880562.

Comment 8

[Dianna Smith [:diannaS]]

Fixed for 115.9esr by 1880562 which was just uplifted to esr115

Comment 9

[Daniel Veditz [:dveditz]]

Making Firefox 122 security bugs public. [bugspam filter string: Pilgarlic-Towers]