Closed Bug 2027324 Opened 6 months ago Closed 5 months ago

NSS_CMSContentInfo_SetContent should take ownership of ptr iff it returns SECSuccess

Categories

(NSS :: Libraries, defect, P2)

Tracking

(nss 3.123, firefox-esr115 wontfix, firefox-esr140 wontfix, firefox149 wontfix, firefox150 wontfix, firefox151 fixed)

RESOLVED FIXED
Tracking Status
nss --- 3.123
firefox-esr115 --- wontfix
firefox-esr140 --- wontfix
firefox149 --- wontfix
firefox150 --- wontfix
firefox151 --- fixed

People

(Reporter: bugmon, Assigned: keeler)

Details

(5 keywords, Whiteboard: [prefs-checked][adv-main151+r])

Attachments

(5 files)

Summary

Heap-use-after-free in nsNSSCertificateDB::AsPKCS7Blob (security/manager/ssl/nsNSSCertificateDB.cpp:1134-1141). When NSS_CMSContentInfo_SetContent_SignedData fails due to OOM after NSS has already stored the sigd pointer in cmsg->contentInfo.content.pointer, the early-return at line 1138 skips sigd.release(). During stack unwind, ~UniqueNSSCMSSignedData and ~UniqueNSSCMSMessage both invoke NSS_CMSSignedData_Destroy on the same arena-resident structure, walking sigd->certs[] twice and calling CERT_DestroyCertificate on each entry twice. The extra refcount decrement frees the CERTCertificate while nsNSSCertificate::mCert still holds a raw pointer to it, leading to UAF when ~nsNSSCertificate later runs.

Affected Code

File: security/manager/ssl/nsNSSCertificateDB.cpp, lines 1106-1141

UniqueNSSCMSMessage cmsg(NSS_CMSMessage_Create(nullptr));   // line 1106
...
UniqueNSSCMSSignedData sigd(nullptr);                       // line 1113
for (const auto& cert : certList) {
  UniqueCERTCertificate nssCert(cert->GetCert());
  if (!sigd) {
    sigd.reset(
        NSS_CMSSignedData_CreateCertsOnly(cmsg.get(), nssCert.get(), false));
    // ^ CERT_DupCertificate stores cert in arena-allocated sigd->certs[]
    ...
  }
  ...
}

NSSCMSContentInfo* cinfo = NSS_CMSMessage_GetContentInfo(cmsg.get());
if (NSS_CMSContentInfo_SetContent_SignedData(cmsg.get(), cinfo, sigd.get()) !=
    SECSuccess) {
  MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
          ("nsNSSCertificateDB::AsPKCS7Blob - can't attach SignedData"));
  return NS_ERROR_FAILURE;   // ← BUG: sigd.release() skipped, but cmsg already owns sigd
}
// cmsg owns sigd now.
(void)sigd.release();        // line 1141 — only reached on success

Why the code is vulnerable: The Firefox code assumes that if NSS_CMSContentInfo_SetContent_SignedData returns SECFailure, ownership of sigd was not transferred. But the NSS implementation (security/nss/lib/smime/cmscinfo.c:155-190) assigns cinfo->content.pointer = ptr at line 175 before the fallible SECITEM_AllocItem(cmsg->poolp, NULL, 1) at line 183:

cinfo->contentTypeTag = SECOID_FindOIDByTag(type);   // set to SIGNED_DATA
...
cinfo->content.pointer = ptr;                        // ← OWNERSHIP TRANSFERRED HERE

if (NSS_CMSType_IsData(type) && ptr) {
    cinfo->rawContent = ptr;
} else {
    cinfo->rawContent = SECITEM_AllocItem(cmsg->poolp, NULL, 1);  // ← CAN FAIL (OOM)
    if (cinfo->rawContent == NULL) {
        PORT_SetError(SEC_ERROR_NO_MEMORY);
        return SECFailure;    // ← but content.pointer still = sigd, contentTypeTag still = SIGNED_DATA
    }
}

When the failure path returns, sigd is aliased by both the UniqueNSSCMSSignedData stack local and cmsg->contentInfo.content.signedData.

The double-destroy chain works because sigd is arena-allocated in cmsg->poolp (cmssigdata.c:35: PORT_ArenaZAlloc(poolp, sizeof(NSSCMSSignedData))), and the arena is only freed at the very end of NSS_CMSMessage_Destroy (cmsmessage.c:116). Meanwhile NSS_CMSSignedData_Destroy (cmssigdata.c:67-70) walks sigd->certs[] calling CERT_DestroyCertificate but never nulls sigd->certs — so the second invocation walks the same pointers again.

Correct Pattern

NSSCMSContentInfo* cinfo = NSS_CMSMessage_GetContentInfo(cmsg.get());
// NSS_CMSContentInfo_SetContent stores sigd in cmsg->contentInfo.content.pointer
// BEFORE its internal fallible allocation. On any return (success or failure),
// cmsg may already own sigd. Release BEFORE the call, and on failure manually
// clear the stored pointer to prevent cmsg's destructor from destroying it twice.
NSSCMSSignedData* rawSigd = sigd.release();
if (NSS_CMSContentInfo_SetContent_SignedData(cmsg.get(), cinfo, rawSigd) !=
    SECSuccess) {
  // If NSS stored the pointer before failing, cmsg->contentInfo now aliases
  // rawSigd and cmsg's destructor will free it. If NSS failed BEFORE storing
  // (e.g. SECITEM_CopyItem failure), the pointer was never stored and we must
  // free it ourselves. Check whether ownership was transferred.
  if (cinfo->content.signedData != rawSigd) {
    NSS_CMSSignedData_Destroy(rawSigd);
  }
  MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
          ("nsNSSCertificateDB::AsPKCS7Blob - can't attach SignedData"));
  return NS_ERROR_FAILURE;
}
// cmsg owns sigd now — fall through.

Alternatively, NSS itself should be fixed to either (a) move cinfo->content.pointer = ptr to the end of NSS_CMSContentInfo_SetContent after all fallible operations, or (b) null out content.pointer and contentTypeTag on the failure path before returning.

Exploit Chain

  1. User opens Preferences → Privacy & Security → Certificates → View Certificates, selects a certificate, and clicks Export → PKCS#7 (via getPKCS7Array at pippki.sys.mjs:51-61).
  2. Chrome JS calls certdb.asPKCS7Blob([cert]) → nsNSSCertificateDB::AsPKCS7Blob in the parent process.
  3. NSS_CMSMessage_Create allocates cmsg with its own arena pool. UniqueNSSCMSMessage cmsg takes ownership.
  4. NSS_CMSSignedData_CreateCertsOnly arena-allocates sigd in cmsg->poolp and calls CERT_DupCertificate(cert) → cert refcount goes from 1 (held by nsNSSCertificate::mCert) to 2 (local nssCert) to 3 (stored in sigd->certs[0]), then back to 2 when nssCert goes out of scope. UniqueNSSCMSSignedData sigd takes ownership.
  5. NSS_CMSContentInfo_SetContent_SignedData → NSS_CMSContentInfo_SetContent:
    • Sets cinfo->contentTypeTag = SEC_OID_PKCS7_SIGNED_DATA
    • Sets cinfo->content.pointer = sigd
    • Calls SECITEM_AllocItem(cmsg->poolp, NULL, 1) → OOM → returns NULL
    • Returns SECFailure with cinfo->content.signedData still pointing at sigd
  6. Line 1138: return NS_ERROR_FAILURE — sigd.release() at line 1141 is skipped.
  7. Stack unwind, destructor #1 (sigd declared after cmsg → destroyed first): ~UniqueNSSCMSSignedData → NSS_CMSSignedData_Destroy(sigd) → walks sigd->certs[] → CERT_DestroyCertificate(cert) → refcount 2→1. sigd->certs not nulled (arena memory).
  8. Stack unwind, destructor #2: ~UniqueNSSCMSMessage → NSS_CMSMessage_Destroy(cmsg) → NSS_CMSContentInfo_Destroy(&cmsg->contentInfo) → switch on kind (= SEC_OID_PKCS7_SIGNED_DATA from step 5) → NSS_CMSSignedData_Destroy(cinfo->content.signedData) — same sigd, still valid (arena not yet freed) → walks sigd->certs[] again → CERT_DestroyCertificate(cert) → refcount 1→0 → nssCertificate_Destroy → nssDecodedPKIXCertificate_Destroy → PORT_FreeArena → cert freed. Then PORT_FreeArena(cmsg->poolp) finally frees sigd's backing memory.
  9. nsNSSCertificate::mCert is now dangling. When the nsNSSCertificate XPCOM object is later released (e.g. when chrome JS drops its reference, or at GC), ~nsNSSCertificate → ~DataMutexBase → ~Maybe<UniqueCERTCertificate> → ~UniqueCERTCertificate → CERT_DestroyCertificate(dangling) → reads cert->nssCertificate at stanpcertdb.c:830 → heap-use-after-free.

Steps to Reproduce

Because the natural trigger requires OOM at a ~25-byte arena allocation during user-driven cert export, reproduction requires simulating the OOM:

  1. Apply the NSS patch to security/nss/lib/smime/cmscinfo.c that forces SECFailure / SEC_ERROR_NO_MEMORY for SEC_OID_PKCS7_SIGNED_DATA after content.pointer is set (models SECITEM_AllocItem OOM).
  2. Apply the FuzzingFunctions::TestCertDBExport shim to dom/base/FuzzingFunctions.{cpp,h} and dom/webidl/FuzzingFunctions.webidl (calls unmodified AsPKCS7Blob with a test cert).
  3. Build with --enable-fuzzing and ASAN.
  4. Set pref fuzzing.enabled = true.
  5. Load the HTML testcase calling FuzzingFunctions.testCertDBExport().

Security Impact

Severity: sec-low. This is a genuine heap-use-after-free in parent-process memory, but reaching it requires:

  • User interaction: The only production caller is the Certificate Export UI (pippki.sys.mjs), invoked when a user manually exports certificates as PKCS#7 from Firefox Preferences. There is no web-content or IPC path to nsNSSCertificateDB::AsPKCS7Blob — the service is ParentProcessOnly.
  • Precisely timed OOM: SECITEM_AllocItem(cmsg->poolp, NULL, 1) must fail. This is a ~25-byte arena allocation (sizeof(SECItem) + 1) from a freshly-created arena pool. An attacker cannot directly induce OOM at this exact instant from untrusted input.

If triggered, the UAF reads cert->nssCertificate (a pointer at offset 696 in the freed CERTCertificate arena) and, if non-null, calls NSSCertificate_Destroy on it — a write-what-where primitive gated on heap grooming between the double-destroy (step 8) and ~nsNSSCertificate (step 9). In the parent process this would be a privilege-escalation primitive, but the trigger preconditions make practical exploitation implausible. This should be fixed as a defense-in-depth robustness issue.

ASAN Report

==175240==ERROR: AddressSanitizer: heap-use-after-free on address 0x7d5dd353cf38 at pc 0x7b8dceb5a9b8 bp 0x7ffdb743e960 sp 0x7ffdb743e958
READ of size 8 at 0x7d5dd353cf38 thread T0 (Isolated Web Co)
    #0 0x7b8dceb5a9b7 in CERT_DestroyCertificate /firefox/security/nss/lib/certdb/stanpcertdb.c:830:37
    #1 0x7b8db605430e in mozilla::UniqueCERTCertificateDeletePolicy::operator()(CERTCertificateStr*) /firefox/security/manager/ssl/ScopedNSSTypes.h:387:1
    #2 0x7b8db605430e in std::unique_ptr<CERTCertificateStr, mozilla::UniqueCERTCertificateDeletePolicy>::~unique_ptr() /root/.mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/unique_ptr.h:361:4
    #3 0x7b8db605430e in mozilla::detail::MaybeStorage<std::unique_ptr<CERTCertificateStr, mozilla::UniqueCERTCertificateDeletePolicy>, false>::~MaybeStorage() /firefox/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Maybe.h:269:25
    #4 0x7b8db605430e in mozilla::DataMutexBase<mozilla::Maybe<std::unique_ptr<CERTCertificateStr, mozilla::UniqueCERTCertificateDeletePolicy>>, mozilla::Mutex>::~DataMutexBase() /firefox/obj-x86_64-pc-linux-gnu/dist/include/mozilla/DataMutex.h:37:7
    #5 0x7b8db605430e in nsNSSCertificate::~nsNSSCertificate() /firefox/security/manager/ssl/nsNSSCertificate.h:32:31
    #6 0x7b8db605430e in nsNSSCertificate::Release() /firefox/security/manager/ssl/nsNSSCertificate.cpp:54:1
    #7 0x7b8dacabefbd in mozilla::RefPtrTraits<nsIX509Cert>::Release(nsIX509Cert*) /firefox/obj-x86_64-pc-linux-gnu/dist/include/mozilla/RefPtr.h:47:40
    #8 0x7b8dacabefbd in nsCOMPtr<nsIX509Cert>::~nsCOMPtr() /firefox/xpcom/base/nsCOMPtr.h:342:7
    #9 0x7b8dacabefbd in mozilla::dom::FuzzingFunctions::TestCertDBExport(mozilla::dom::GlobalObject const&) /firefox/dom/base/FuzzingFunctions.cpp:524:1
    ...

0x7d5dd353cf38 is located 696 bytes inside of 2048-byte region [0x7d5dd353cc80,0x7d5dd353d480)
freed by thread T0 (Isolated Web Co) here:
    #0 0x57bd489f66f6 in __interceptor_free _asan_rtl_:3
    #1 0x7f8dd3b27cc7 in FreeArenaList /firefox/nsprpub/lib/ds/plarena.c:201:5
    #2 0x7f8dd3b27cc7 in PL_FreeArenaPool /firefox/nsprpub/lib/ds/plarena.c:220:3
    #3 0x7f8dd37a9c08 in PORT_FreeArena_Util /firefox/security/nss/lib/util/secport.c:418:9
    #4 0x7b8dceba6480 in nssDecodedPKIXCertificate_Destroy /firefox/security/nss/lib/pki/pki3hack.c:560:9
    #5 0x7b8dceba051f in nssCertificate_Destroy /firefox/security/nss/lib/pki/certificate.c:123:13
    #6 0x7b8dceba0668 in NSSCertificate_Destroy /firefox/security/nss/lib/pki/certificate.c:141:12
    #7 0x7b8dcea995c9 in NSS_CMSSignedData_Destroy /firefox/security/nss/lib/smime/cmssigdata.c:69:13
    #8 0x7b8dcea8c5d4 in NSS_CMSContentInfo_Destroy /firefox/security/nss/lib/smime/cmscinfo.c:64:13
    #9 0x7b8dcea95926 in NSS_CMSMessage_Destroy /firefox/security/nss/lib/smime/cmsmessage.c:112:5
    #10 0x7b8db606002b in mozilla::UniqueNSSCMSMessageDeletePolicy::operator()(NSSCMSMessageStr*) /firefox/security/manager/ssl/ScopedNSSTypes.h:415:1
    #11 0x7b8db606002b in std::unique_ptr<NSSCMSMessageStr, mozilla::UniqueNSSCMSMessageDeletePolicy>::~unique_ptr() /root/.mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/unique_ptr.h:361:4
    #12 0x7b8db606002b in nsNSSCertificateDB::AsPKCS7Blob(nsTArray<RefPtr<nsIX509Cert>> const&, nsTSubstring<char>&) /firefox/security/manager/ssl/nsNSSCertificateDB.cpp:1169:1
    #13 0x7b8dacabee28 in mozilla::dom::FuzzingFunctions::TestCertDBExport(mozilla::dom::GlobalObject const&) /firefox/dom/base/FuzzingFunctions.cpp:519:17
    ...

previously allocated by thread T0 (Isolated Web Co) here:
    #0 0x57bd489f6994 in __interceptor_malloc _asan_rtl_:3
    #1 0x7f8dd3b27653 in PL_ArenaAllocate /firefox/nsprpub/lib/ds/plarena.c:132:21
    #2 0x7f8dd37a9957 in PORT_ArenaAlloc_Util /firefox/security/nss/lib/util/secport.c:356:13
    #3 0x7f8dd37a9b38 in PORT_ArenaZAlloc_Util /firefox/security/nss/lib/util/secport.c:377:9
    #4 0x7b8dceb404e0 in CERT_DecodeDERCertificate /firefox/security/nss/lib/certdb/certdb.c:736:31
    #5 0x7b8dceba71cf in nssDecodedPKIXCertificate_Create /firefox/security/nss/lib/pki/pki3hack.c:496:12
    #6 0x7b8dceba71cf in stan_GetCERTCertificate /firefox/security/nss/lib/pki/pki3hack.c:919:14
    #7 0x7b8dceb5c12a in CERT_NewTempCertificate /firefox/security/nss/lib/certdb/stanpcertdb.c:416:10
    #8 0x7b8db605f070 in nsNSSCertificateDB::ConstructX509FromSpan(mozilla::Span<unsigned char const, 18446744073709551615ul>, nsIX509Cert**) /firefox/security/manager/ssl/nsNSSCertificateDB.cpp:899:30
    ...

SUMMARY: AddressSanitizer: heap-use-after-free (/firefox/obj-x86_64-pc-linux-gnu/dist/bin/libnss3.so+0xa89b7)

Suggested Fix

The clean fix is in NSS — make NSS_CMSContentInfo_SetContent transactional so partial state is never left on failure:

/* security/nss/lib/smime/cmscinfo.c — NSS_CMSContentInfo_SetContent */
SECStatus
NSS_CMSContentInfo_SetContent(NSSCMSMessage *cmsg, NSSCMSContentInfo *cinfo,
                              SECOidTag type, void *ptr)
{
    SECStatus rv;
    SECOidData *typeTag;
    SECItem *rawContent = NULL;

    if (cinfo == NULL || cmsg == NULL) {
        return SECFailure;
    }

    typeTag = SECOID_FindOIDByTag(type);
    if (typeTag == NULL) {
        return SECFailure;
    }

    /* Perform ALL fallible work BEFORE mutating cinfo. */
    if (NSS_CMSType_IsData(type) && ptr) {
        rawContent = ptr;
    } else {
        rawContent = SECITEM_AllocItem(cmsg->poolp, NULL, 1);
        if (rawContent == NULL) {
            PORT_SetError(SEC_ERROR_NO_MEMORY);
            return SECFailure;           /* nothing to roll back */
        }
    }

    rv = SECITEM_CopyItem(cmsg->poolp, &(cinfo->contentType), &(typeTag->oid));
    if (rv != SECSuccess) {
        return SECFailure;               /* rawContent is arena-owned, leaks with arena */
    }

    /* Commit: no failures possible past this point. */
    cinfo->contentTypeTag = typeTag;
    cinfo->content.pointer = ptr;
    cinfo->rawContent = rawContent;
    return SECSuccess;
}

Defense-in-depth on the Firefox side at nsNSSCertificateDB.cpp:1134:

// Release BEFORE the call: NSS may store sigd in cmsg->contentInfo even on failure.
NSSCMSSignedData* rawSigd = sigd.release();
if (NSS_CMSContentInfo_SetContent_SignedData(cmsg.get(), cinfo, rawSigd) !=
    SECSuccess) {
  // If NSS failed before storing, we still own rawSigd and must free it.
  // If NSS failed after storing, cmsg's destructor will free it — don't double-free.
  if (cinfo->content.signedData != rawSigd) {
    NSS_CMSSignedData_Destroy(rawSigd);
  }
  return NS_ERROR_FAILURE;
}
Attached file crash_stack.txt —
Attached file REPRODUCTION_STEPS.md —
Group: core-security → dom-core-security
Whiteboard: [prefs-checked]

fuzzing.enabled is only for the FuzzingFunctions test shim — the vulnerable AsPKCS7Blob path is reachable in production via the cert export UI (pippki.sys.mjs) with no prefs. Supported config; tagging [prefs-checked].

sec-low looks right: requires manual navigation to Preferences → Certificates → Export-as-PKCS#7 plus an OOM on a ~25-byte arena alloc that an attacker can't influence.

This is an automated analysis result. If this result is incorrect please add a needinfo and feel free to correct the error.

Group: dom-core-security → crypto-core-security
Component: General → Security: PSM
Assignee: nobody → dkeeler
Severity: -- → S4
Component: Security: PSM → Libraries
Product: Core → NSS
Summary: Heap-use-after-free in [@ nsNSSCertificateDB::AsPKCS7Blob] via double NSS_CMSSignedData_Destroy on SetContent OOM → NSS_CMSContentInfo_SetContent should take ownership of ptr iff it returns SECSuccess
Version: Trunk → unspecified
Attached file (secure) —
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Pushed by jschanck@mozilla.com:
https://hg.mozilla.org/projects/nss/rev/3420ee86581e
NSS_CMSContentInfo_SetContent: only modify cinfo if everything succeeds r=nss-reviewers,jschanck

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Group: crypto-core-security → core-security-release
QA Whiteboard: [sec] [qa-triage-done-c152/b151]
Whiteboard: [prefs-checked] → [prefs-checked][adv-main151+r][adv-esr115.36+r][adv-esr140.11+r]
Whiteboard: [prefs-checked][adv-main151+r][adv-esr115.36+r][adv-esr140.11+r] → [prefs-checked][adv-main151+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: