1748105 - Double-free NSS Crash in [@ je_free | PL_FreeArenaPool | cert_GetCertificateEmailAddresses | CERT_DecodeDERCertificate | stan_GetCERTCertificate]

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

Double-free crash; goes back to at least 76.

Marking as security because NSS

Crash report: https://crash-stats.mozilla.org/report/index/a203b8ec-2e8e-4963-abcb-196d80211225

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT((run->mRegionsMask[elm] & (1U << bit)) == 0) (Double-free?)

Top 10 frames of crashing thread:

0 mozglue.dll je_free memory/build/malloc_decls.h:54
1 nss3.dll PL_FreeArenaPool nsprpub/lib/ds/plarena.c:229
2 nss3.dll cert_GetCertificateEmailAddresses security/nss/lib/certdb/alg1485.c:1483
3 nss3.dll CERT_DecodeDERCertificate security/nss/lib/certdb/certdb.c:793
4 nss3.dll stan_GetCERTCertificate security/nss/lib/pki/pki3hack.c:895
5 nss3.dll CERT_NewTempCertificate security/nss/lib/certdb/stanpcertdb.c:411
6 nss3.dll CERT_DecodeCertFromPackage security/nss/lib/pkcs7/certread.c:522
7 xul.dll nsNSSCertificate::Read security/manager/ssl/nsNSSCertificate.cpp:694
8 xul.dll nsBinaryInputStream::ReadObject xpcom/io/nsBinaryStream.cpp:940
9 xul.dll mozilla::psm::TransportSecurityInfo::Read security/manager/ssl/TransportSecurityInfo.cpp:568

Comment 1

[John Schanck [:jschanck]]

I'm adding another signature which shows some evidence of memory corruption while processing an X.509 Name. This is a bit concerning since it might be easy to trigger remotely by sending a specially crafted certificate.

I haven't been able to track down the exact cause. My current theory is that there is an error in cert_RFC1485_GetRequiredLen and/or escapeAndQuote. These two functions are called by AppendAVA (which is called shortly before the CERT_NameToAsciiInvertible crash) and avaToString (which is called shortly before the cert_GetCertificateEmailAddresses crash).

These functions would be good candidates for fuzzing.

Comment 2

[mozillabugs]

(In reply to John Schanck [:jschanck] from comment #1)

I'm adding another signature which shows some evidence of memory corruption while processing an X.509 Name. This is a bit concerning since it might be easy to trigger remotely by sending a specially crafted certificate.

I haven't been able to track down the exact cause. My current theory is that there is an error in cert_RFC1485_GetRequiredLen and/or escapeAndQuote. These two functions are called by AppendAVA (which is called shortly before the CERT_NameToAsciiInvertible crash) and avaToString (which is called shortly before the cert_GetCertificateEmailAddresses crash)....

Well, escapeAndQuote() does

[int]     reqLen = cert_RFC1485_GetRequiredLen(src, srclen, &mode) + 1;

But cert_RFC1485_GetRequiredLen() can experience an integer overflow, since it can compute a value as large as srclen*3 + 2 and srclen isa int.

On return, escapeAndQuote() does

    if (reqLen > dstlen) {
        PORT_SetError(SEC_ERROR_OUTPUT_LEN);
        return SECFailure;
    }

But if reqLen has overflowed to negative, the condition will be false and escapeAndQuote() will write beyond bounds. Once that happens, all bets are off and you could get a "double free?" assertion or worse.(Your mention of "evidence of memory corruption" is also consistent with this mechanism). Also reqLen could overflow to a small positive value, with the same consequences. The minimum value of srclen that can cause the overflow is 0x80000000 - 2 / 3 == 0x2aaaaaaa. This is well within the maximum size of an nsCString, an nsTArray<uint8_t>, an std::string, etc. I don't know whether some other size check on this path prevents such a large srclen from getting to this code.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Good analysis. I've made cert_RFC1485_GetRequiredLen() detect overflow and added an error return (and checks for it)

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 1748105: clean up escape handling r=jschanck!

Comment 5

[John Schanck [:jschanck]]

The one crash report we have traces back to here in ssl_DecodeResumptionToken. The certificate length is encoded in 3 bytes, so readerBuffer.len is no more than 2^24. The input to escapeAndQuote, assuming it is constructed correctly, is a subslice of readerBuffer. So 3*srclen+2 will not overflow.

Comment 6

[John Schanck [:jschanck]]

Attachment: bug1748105.crt.gz

A certificate with a ~1.4 GB email address in its subject. To reproduce crash: certutil -A -d. -t,, -n bug1748105 -i bug1748105.crt

Comment 7

[mozillabugs]

(In reply to John Schanck [:jschanck] from comment #6)

Created attachment 9401711 [details]
bug1748105.crt.gz

A certificate with a ~1.4 GB email address in its subject. To reproduce crash: certutil -A -d. -t,, -n bug1748105 -i bug1748105.crt

Nice: write beyond bounds in escapeAndQuote().

Comment 8

[John Schanck [:jschanck]]

It occurred to me this morning that we might be able to reach this code during addon signature verification, so I constructed an XPI file with a certificate similar to the one I posted above. Fortunately it seems that we have an 8MB cap on the decompressed size of XPI file contents: https://searchfox.org/mozilla-central/rev/55b8c839700564409af5295286a1fd389410e19b/security/manager/ssl/AppSignatureVerification.cpp#108-111.

Comment 9

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/4c6b64191c6d5039cda08b1c6233c5c33f008858

Comment 10

[mozillabugs]

Comment 2 identified the cause of the bug, so I'm requesting sec-bounty.

Comment 11

[Frederik Braun [:freddy]]

Yeah, thanks for providing the analysis in comment 2. We are awarding the bounty on the lower end of the "sec high" spectrum, since this wasn't directly found by you. But in the end, your root causing has been really helpful enough to be as valuable as any other sec-high to us.

Comment 12

[Donal Meehan [:dmeehan]]

Setting esr115 to Fixed for 115.13esr.
Bug 1905160 was uplifted to esr115