1421788 - Crash in nssCryptokiObject_Create | create_objects_from_handles

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-ea3524f7-c05b-4775-a680-1fc170171129.
=============================================================

Top 10 frames of crashing thread:

0 nss3.dll nssCryptokiObject_Create security/nss/lib/dev/devutil.c:43
1 nss3.dll create_objects_from_handles security/nss/lib/dev/devtoken.c:230
2 nss3.dll find_objects security/nss/lib/dev/devtoken.c:336
3 nss3.dll nssToken_FindObjectsByTemplate security/nss/lib/dev/devtoken.c:425
4 nss3.dll nssToken_FindCertificatesBySubject security/nss/lib/dev/devtoken.c:620
5 nss3.dll nssTrustDomain_FindCertificatesBySubject security/nss/lib/pki/trustdomain.c:614
6 nss3.dll CERT_CreateSubjectCertList security/nss/lib/certdb/stanpcertdb.c:744
7 xul.dll mozilla::psm::NSSCertDBTrustDomain::FindIssuer security/certverifier/NSSCertDBTrustDomain.cpp:154
8 xul.dll mozilla::pkix::BuildForward security/pkix/lib/pkixbuild.cpp:340
9 xul.dll mozilla::pkix::BuildCertChain security/pkix/lib/pkixbuild.cpp:376

=============================================================

this signature is regressing in numbers in the firefox 58 cycle. overall it's still rather low-to-mid volume though. mostly 32bit builds of firefox on windows seem to be affected.

Comment 1

[Dana Keeler (she/her) [:keeler]]

Here's the code in question:

41 isTokenObject = (CK_BBOOL *)cert_template[0].pValue;
42 object->isTokenObject = *isTokenObject;
43 nss_ZFreeIf(isTokenObject);

The stack trace is pointing to line 43, but I think on line 42 we're dereferencing a pointer without checking it first. All other similar code first either checks the length of the item and/or the pointer itself. I'll put up a speculative patch.

Comment 2

[Phabricator Automation]

Comment on attachment 8933122 [details]
bug 1421788 - add a length check in nssCryptokiObject_Create to maybe prevent null pointer deref r?ttaubert

Tim Taubert [:ttaubert] has approved the revision.

https://phabricator.services.mozilla.com/D302#7517

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/0c062fe18d23

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Fx58 crash volume doesn't seem worth a 3.34 backport and uplift, so calling 58 wontfix. Feel free to set it back to affected and spin a new release if you feel strongly otherwise, though :)

Comment 5

[Chris Newman]

We're seeing this regression crash in production. Any chance the fix could go into an NSS 3.34.2 release soon?