2029791 - Heap-buffer-overflow READ in [@ PK11_FindCrlByName] via PORT_Strdup of unterminated CKA_NSS_URL

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[Bugmon [:jkratzer for issues]]

PK11_FindCrlByName (security/nss/lib/pk11wrap/pk11nobj.c:355) calls PORT_Strdup on crl->url, which is an NSSUTF8* pointing to a buffer that is not NUL-terminated. PORT_Strdup invokes strlen, which reads past the end of the URL allocation into adjacent arena memory, then memcpy's the over-read bytes into a fresh heap allocation that is returned to the caller as the CRL's URL string.

The root cause is a contract mismatch between the PKCS#11 attribute storage and retrieval layers. On the write side, nssToken_ImportCRL (devtoken.c:1196) uses NSS_CK_SET_ATTRIBUTE_UTF8, which deliberately sets ulValueLen = strlen(url), excluding the NUL terminator (consistent with PKCS#11 RFC 2279 string conventions). On the read side, nssCKObject_GetAttributes (ckhelper.c:101-104) is supposed to compensate by allocating ulValueLen+1 bytes for string attributes — but is_string_attribute() (ckhelper.c:38-53) only recognises CKA_LABEL and CKA_NSS_EMAIL. CKA_NSS_URL is missing from this whitelist, so the URL buffer is allocated with exactly strlen(url) bytes and no NUL slot. NSS_CK_ATTRIBUTE_TO_UTF8 then casts the raw pointer to NSSUTF8* with no length check, and nssCRL_Create (certificate.c:1080) stores it directly in crl->url.

The bug is reachable through the exported public API SEC_NewCrl (nss.def:146) and PK11_ImportCRL (nss.def:682, used by crlutil). Importing a CRL with any URL stores it without a NUL terminator on the token; importing a second CRL for the same issuer triggers crl_storeCRL → SEC_FindCrlByKeyOnSlot → PK11_FindCrlByName, which strlen's the unterminated buffer. No malformed input is required — a perfectly valid URL such as "http://example.com/A" triggers the over-read. The leaked bytes (typically arena alignment padding plus the next allocation's pointer_header containing a heap pointer) propagate into the returned crl->url string and may be surfaced to applications that log or display CRL URLs.

Build Info

• Branch: main
• Revision: 6164ea4bacaeaed1f617c11911df7fc32f2e6ec2
• Timestamp: 2026-04-02T19:36:24+00:00

Affected Code

File: security/nss/lib/pk11wrap/pk11nobj.c, line 354-358

    if (crl->url) {
        url = PORT_Strdup(crl->url);   /* strlen() over-reads unterminated buffer */
        if (!url) {
            goto loser;
        }
    }

File: security/nss/lib/dev/ckhelper.c, line 38-53

static PRBool
is_string_attribute(
    CK_ATTRIBUTE_TYPE aType)
{
    PRBool isString;
    switch (aType) {
        case CKA_LABEL:
        case CKA_NSS_EMAIL:
            isString = PR_TRUE;
            break;
        default:                    /* CKA_NSS_URL falls through here -> no +1 NUL byte */
            isString = PR_FALSE;
            break;
    }
    return isString;
}

File: security/nss/lib/dev/ckhelper.c, line 94-109

        for (i = 0; i < count; i++) {
            CK_ULONG ulValueLen = obj_template[i].ulValueLen;
            if (ulValueLen == 0 || ulValueLen == (CK_ULONG)-1) {
                obj_template[i].pValue = NULL;
                obj_template[i].ulValueLen = 0;
                continue;
            }
            if (is_string_attribute(obj_template[i].type)) {
                ulValueLen++;            /* never executed for CKA_NSS_URL */
            }
            obj_template[i].pValue = nss_ZAlloc(arenaOpt, ulValueLen);  /* exact size, no NUL slot */
            if (!obj_template[i].pValue) {
                nssSession_ExitMonitor(session);
                goto loser;
            }
        }

File: security/nss/lib/dev/ckhelper.h, line 39-45

#define NSS_CK_SET_ATTRIBUTE_UTF8(pattr, kind, utf8)          \
    (pattr)->type = kind;                                     \
    (pattr)->pValue = (CK_VOID_PTR)utf8;                      \
    (pattr)->ulValueLen = (CK_ULONG)nssUTF8_Size(utf8, NULL); \
    if ((pattr)->ulValueLen)                                  \
        ((pattr)->ulValueLen)--;       /* explicitly drops the NUL: stored len = strlen(url) */ \
    (pattr)++;

File: security/nss/lib/dev/ckhelper.h, line 100-101

#define NSS_CK_ATTRIBUTE_TO_UTF8(attrib, str) \
    str = (NSSUTF8 *)((attrib)->pValue);    /* raw cast, no termination check */

File: security/nss/lib/dev/ckhelper.c, line 514-516, 556-558

    if (urlOpt) {
        NSS_CK_SET_ATTRIBUTE_NULL(attr, CKA_NSS_URL);   /* template entry, ulValueLen=0 */
    }
    /* ... nssCKObject_GetAttributes allocates with no NUL ... */
    if (urlOpt) {
        NSS_CK_ATTRIBUTE_TO_UTF8(&crl_template[i], *urlOpt);  /* assigns unterminated ptr to crl->url */
        i++;
    }

File: security/nss/lib/pki/certificate.c, line 1074-1081

    status = nssCryptokiCRL_GetAttributes(object->instances[0],
                                          NULL, /* XXX sessionOpt */
                                          arena,
                                          &rvCRL->encoding,
                                          NULL, /* subject */
                                          NULL, /* class */
                                          &rvCRL->url,        /* <-- receives unterminated pointer */
                                          &rvCRL->isKRL);

File: security/nss/lib/util/secport.c, line 186-196

char *
PORT_Strdup(const char *str)
{
    size_t len = PORT_Strlen(str) + 1;   /* strlen reads past end of buffer */
    char *newstr;

    newstr = (char *)PORT_Alloc(len);
    if (newstr) {
        PORT_Memcpy(newstr, str, len);    /* copies leaked bytes into fresh allocation */
    }
    return newstr;
}

The bug is a write-side/read-side contract mismatch for the CKA_NSS_URL PKCS#11 attribute. The write side stores strlen(url) bytes (no NUL) per PKCS#11 string convention. The read side has a special-case to add a NUL slot for string attributes, but the whitelist in is_string_attribute() forgot CKA_NSS_URL. The unterminated buffer is then handed to PORT_Strdup as if it were a C string. The fix is a one-line addition of case CKA_NSS_URL: to is_string_attribute().

Exploit Chain

1. Caller invokes SEC_NewCrl(handle, "http://example.com/A", &derCrl, SEC_CRL_TYPE) — or any equivalent CRL import with a URL whose strlen does not coincidentally land on an arena alignment boundary that happens to contain a zero byte.
2. PK11_ImportCRL → crl_storeCRL → PK11_PutCrl → nssToken_ImportCRL stores CKA_NSS_URL on the softoken with ulValueLen = strlen(url) = 20 bytes, NUL excluded (devtoken.c:1196 via NSS_CK_SET_ATTRIBUTE_UTF8).
3. Caller invokes SEC_NewCrl again for the same issuer (e.g. periodic CRL refresh, or any code path that re-imports a CRL).
4. PK11_ImportCRL → crl_storeCRL unconditionally calls SEC_FindCrlByKeyOnSlot at crl.c:600 to look for an existing CRL with the same issuer name.
5. SEC_FindCrlByKeyOnSlot → PK11_FindCrlByName finds the stored CRL token object and calls nssPKIObjectCollection_GetCRLs → nssCRL_Create → nssCryptokiCRL_GetAttributes.
6. nssCryptokiCRL_GetAttributes builds a template containing CKA_NSS_URL and calls nssCKObject_GetAttributes. The first C_GetAttributeValue returns ulValueLen = 20. is_string_attribute(CKA_NSS_URL) returns PR_FALSE, so nss_ZAlloc(arena, 20) is called — exactly 20 bytes, no NUL slot. The second C_GetAttributeValue fills bytes [0..19] with URL data; byte [20] is unwritten arena padding.
7. NSS_CK_ATTRIBUTE_TO_UTF8 casts pValue to NSSUTF8* and assigns it to crl->url. The pointer points to 20 valid bytes followed by uninitialised arena padding and the next arena allocation's pointer_header (containing a live heap pointer).
8. Back in PK11_FindCrlByName at line 355: PORT_Strdup(crl->url) → strlen reads bytes [0..19], then continues into byte [20] and beyond until it hits a zero byte. Under ASAN this is the poisoned arena padding (shadow byte 0xf7); under a release build it reads the alignment padding plus the next pointer_header.
9. PORT_Strdup then PORT_Memcpy's the over-read length into a fresh heap allocation. The leaked bytes — typically including a heap arena pointer — are returned via *pUrl and propagate into oldCrl->url, then via crl.c:612-615 / 624-625 may be PORT_ArenaStrdup'd into the new CRL's persistent url field, where the application can later observe them.

Steps to Reproduce

1. Build NSS with AddressSanitizer enabled.
2. Add /firefox/security/nss/gtests/pk11_gtest/pk11_findcrl_url_unittest.cc to the pk11_gtest target.
3. Run: pk11_gtest --gtest_filter=Pk11FindCrlUrlTest.StrdupOverreadOnUnterminatedUrl
4. Observe ASAN abort with READ of size 28 in strlen, called from PORT_Strdup_Util at PK11_FindCrlByName (pk11nobj.c:355).
5. Alternatively, using crlutil against a softoken DB: import a CRL with a -u URL whose strlen produces an unaligned arena slot, then import a second CRL for the same issuer.

Security Impact

• Severity: Moderate
• Attacker capability: Heap memory disclosure. strlen reads beyond the URL allocation into adjacent arena memory, and PORT_Strdup memcpy's the over-read bytes into the returned URL string. In a release build the over-read typically captures arena alignment padding plus the next arena allocation's pointer_header — leaking a live NSSArena* heap pointer (ASLR bypass primitive) appended to the CRL URL string. The leaked data propagates into CERTSignedCrl->url and is observable to any code that logs, displays, or serialises the CRL URL. There is no write primitive; the over-read length is bounded in practice by zero bytes in the next pointer_header's size field (typically tens of bytes), and the leak vector requires the application to surface crl->url to an observer.
• Preconditions: The caller must import a CRL with a non-NULL URL via SEC_NewCrl or PK11_ImportCRL (a normal operation performed by crlutil, browsers managing CRL distribution points, and any NSS consumer that refreshes CRLs), then perform a second import for the same issuer name. No malformed input is required — the URL and CRL DER may be entirely valid. The URL length must merely fail to leave a zero byte at the arena allocation boundary, which is the common case. The PKCS#11 token must not have an object cache for CRLs (true for the internal softoken; dev3hack.c only creates caches for hardware tokens).

ASAN Report

=================================================================
==27991==ERROR: AddressSanitizer: unknown-crash on address 0x51d000034094 at pc 0x7b70ab93996f bp 0x7ffc925627a0 sp 0x7ffc92561f48
READ of size 28 at 0x51d000034094 thread T0
    #0 0x7b70ab93996e in strlen ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:391
    #1 0x7b70ab64eea6 in PORT_Strdup_Util ../../lib/util/secport.c:189
    #2 0x7b70ab78ee3e in PK11_FindCrlByName ../../lib/pk11wrap/pk11nobj.c:355
    #3 0x7b70ab7f5fd6 in SEC_FindCrlByKeyOnSlot ../../lib/certdb/crl.c:532
    #4 0x7b70ab7f64af in crl_storeCRL ../../lib/certdb/crl.c:600
    #5 0x7b70ab791686 in PK11_ImportCRL ../../lib/pk11wrap/pk11nobj.c:721
    #6 0x7b70ab7f6ad5 in SEC_NewCrl ../../lib/certdb/crl.c:670
    #7 0x5a6493cb4812 in nss_test::Pk11FindCrlUrlTest_StrdupOverreadOnUnterminatedUrl_Test::TestBody() ../../gtests/pk11_gtest/pk11_findcrl_url_unittest.cc:137
    #8 0x5a64940451ce in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../../gtests/google_test/gtest/src/gtest.cc:2607
    #9 0x5a64940323bf in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../../gtests/google_test/gtest/src/gtest.cc:2643
    #10 0x5a6493fdb09b in testing::Test::Run() ../../gtests/google_test/gtest/src/gtest.cc:2682
    #11 0x5a6493fdc6c3 in testing::TestInfo::Run() ../../gtests/google_test/gtest/src/gtest.cc:2861
    #12 0x5a6493fddb4e in testing::TestSuite::Run() ../../gtests/google_test/gtest/src/gtest.cc:3015
    #13 0x5a6494004b46 in testing::internal::UnitTestImpl::RunAllTests() ../../gtests/google_test/gtest/src/gtest.cc:5855
    #14 0x5a64940488e0 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../../gtests/google_test/gtest/src/gtest.cc:2607
    #15 0x5a6494035156 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../../gtests/google_test/gtest/src/gtest.cc:2643
    #16 0x5a6494001063 in testing::UnitTest::Run() ../../gtests/google_test/gtest/src/gtest.cc:5438
    #17 0x5a6493fba22f in RUN_ALL_TESTS() ../../gtests/google_test/gtest/include/gtest/gtest.h:2490
    #18 0x5a6493fba5f9 in main ../../gtests/common/gtests.cc:45
    #19 0x7b70aadf81c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #20 0x7b70aadf828a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #21 0x5a649350c6d4 in _start (/firefox/security/dist/Debug/bin/pk11_gtest+0xdb6d4) (BuildId: 77a4d8fa86de1636371ded1edc21e4784660ec21)

0x51d000034094 is located 532 bytes inside of 2048-byte region [0x51d000033e80,0x51d000034680)
allocated by thread T0 here:
    #0 0x7b70ab9b99c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7b70ab38c6e7 in PR_Malloc ../../../../pr/src/malloc/prmem.c:425
    #2 0x7b70ab4289cc in PL_ArenaAllocate ../../../lib/ds/plarena.c:132
    #3 0x7b70ab840ff2 in nss_zalloc_arena_locked ../../lib/base/arena.c:751
    #4 0x7b70ab841471 in nss_ZAlloc ../../lib/base/arena.c:872
    #5 0x7b70ab81d116 in nssPKIObject_Create ../../lib/pki/pkibase.c:101
    #6 0x7b70ab81fca9 in add_object_instance ../../lib/pki/pkibase.c:778
    #7 0x7b70ab820193 in nssPKIObjectCollection_AddInstances ../../lib/pki/pkibase.c:816
    #8 0x7b70ab78eb3f in PK11_FindCrlByName ../../lib/pk11wrap/pk11nobj.c:324
    #9 0x7b70ab7f5fd6 in SEC_FindCrlByKeyOnSlot ../../lib/certdb/crl.c:532
    #10 0x7b70ab7f64af in crl_storeCRL ../../lib/certdb/crl.c:600
    #11 0x7b70ab791686 in PK11_ImportCRL ../../lib/pk11wrap/pk11nobj.c:721
    #12 0x7b70ab7f6ad5 in SEC_NewCrl ../../lib/certdb/crl.c:670
    #13 0x5a6493cb4812 in nss_test::Pk11FindCrlUrlTest_StrdupOverreadOnUnterminatedUrl_Test::TestBody() ../../gtests/pk11_gtest/pk11_findcrl_url_unittest.cc:137
    #14 0x5a64940451ce in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../../gtests/google_test/gtest/src/gtest.cc:2607
    #15 0x5a64940323bf in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../../gtests/google_test/gtest/src/gtest.cc:2643
    #16 0x5a6493fdb09b in testing::Test::Run() ../../gtests/google_test/gtest/src/gtest.cc:2682
    #17 0x5a6493fdc6c3 in testing::TestInfo::Run() ../../gtests/google_test/gtest/src/gtest.cc:2861
    #18 0x5a6493fddb4e in testing::TestSuite::Run() ../../gtests/google_test/gtest/src/gtest.cc:3015
    #19 0x5a6494004b46 in testing::internal::UnitTestImpl::RunAllTests() ../../gtests/google_test/gtest/src/gtest.cc:5855
    #20 0x5a64940488e0 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../../gtests/google_test/gtest/src/gtest.cc:2607
    #21 0x5a6494035156 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../../gtests/google_test/gtest/src/gtest.cc:2643
    #22 0x5a6494001063 in testing::UnitTest::Run() ../../gtests/google_test/gtest/src/gtest.cc:5438
    #23 0x5a6493fba22f in RUN_ALL_TESTS() ../../gtests/google_test/gtest/include/gtest/gtest.h:2490
    #24 0x5a6493fba5f9 in main ../../gtests/common/gtests.cc:45
    #25 0x7b70aadf81c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #26 0x7b70aadf828a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #27 0x5a649350c6d4 in _start (/firefox/security/dist/Debug/bin/pk11_gtest+0xdb6d4) (BuildId: 77a4d8fa86de1636371ded1edc21e4784660ec21)

SUMMARY: AddressSanitizer: unknown-crash ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:391 in strlen
Shadow bytes around the buggy address:
  0x51d000033e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x51d000033e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x51d000033f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x51d000033f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x51d000034000: 00 00 00 00 00 00 00 00 00 00 00 00 00 03 00 00
=>0x51d000034080: 00 00[04]00 00 01 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x51d000034100: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x51d000034180: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x51d000034200: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x51d000034280: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  0x51d000034300: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==27991==ABORTING

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: pk11_gtest.gyp.patch

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: pk11_findcrl_url_unittest.cc

Comment 3

[Bugmon [:jkratzer for issues]]

Attachment: manifest.mn.patch

Comment 4

[Bugmon [:jkratzer for issues]]

Attachment: crash_stack.txt

Comment 6

[Christian Holler (:decoder)]

Attachment: Fix: add CKA_NSS_URL to is_string_attribute() in ckhelper.c

Comment 7

[Christian Holler (:decoder)]

Reproduced the ASAN heap-buffer-overflow READ in strlen via PORT_Strdup at PK11_FindCrlByName using the attached gtest and confirmed this fix resolves it.

Root cause per comment 0 is correct: NSS_CK_SET_ATTRIBUTE_UTF8 stores CKA_NSS_URL with ulValueLen = strlen(url) (no NUL), but is_string_attribute() in lib/dev/ckhelper.c only whitelists CKA_LABEL and CKA_NSS_EMAIL, so nssCKObject_GetAttributes() allocates exactly ulValueLen bytes — no terminator slot — and NSS_CK_ATTRIBUTE_TO_UTF8 hands the unterminated pointer to callers that treat it as a C string (e.g. PORT_Strdup at pk11nobj.c:355).

A tree-wide grep shows the only callers of NSS_CK_SET_ATTRIBUTE_UTF8 use CKA_LABEL, CKA_NSS_EMAIL, or CKA_NSS_URL, so adding CKA_NSS_URL to is_string_attribute() restores symmetry between writer and reader.

Whiteboard: tagged [prefs-checked] — NSS library API, no Firefox prefs involved, supported configuration. sec-moderate looks correct for this info-disclosure over-read.

This is the analysis tool's suggested fix. Feel welcome to adopt it as a starting point and evolve it as needed to meet our coding standards.

Comment 8

[Benjamin Beurdouche [:beurdouche]]

Attachment: (secure)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/nss/rev/465a4ee9d09e
Reserve NUL terminator for CKA_NSS_URL in nssCKObject_GetAttributes. r=nss-reviewers,rrelyea