Closed Bug 1180097 Opened 5 years ago Closed 4 years ago

UBSAN null pointer passed as argument 2, which is declared to never be null


(NSS :: Libraries, defect)

Not set


(firefox42 affected)

Tracking Status
firefox42 --- affected


(Reporter: tsmith, Unassigned)


(Blocks 1 open bug)


(Keywords: crash, csectype-dos)

This was found by running the test suite with an Undefined Behavior Sanitizer (UBSAN) build.

lgattr.c:139:30: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:47:28: note: nonnull attribute specified here

#0 0x7f7f91a18d46 in lg_CopyAttribute /home/user/Desktop/nss_ubsan/nss/lib/softoken/legacydb/lgattr.c:139 (discriminator 11)
#1 0x7f7f91a1f554 in lg_FindPrivateKeyAttribute /home/user/Desktop/nss_ubsan/nss/lib/softoken/legacydb/lgattr.c:1014 (discriminator 6)
#2 0x7f7f91a18244 in lg_GetSingleAttribute /home/user/Desktop/nss_ubsan/nss/lib/softoken/legacydb/lgattr.c:1357
#3 0x7f7f91a2145c in lg_GetAttributeValue /home/user/Desktop/nss_ubsan/nss/lib/softoken/legacydb/lgattr.c:1382
#4 0x7f7f928a8ded in sftkdb_GetAttributeValue /home/user/Desktop/nss_ubsan/nss/lib/softoken/sftkdb.c:1330
#5 0x7f7f9287cff7 in sftk_FindTokenAttribute /home/user/Desktop/nss_ubsan/nss/lib/softoken/pkcs11u.c:130
#6 0x7f7f9287be7f in sftk_FindAttribute /home/user/Desktop/nss_ubsan/nss/lib/softoken/pkcs11u.c:176
#7 0x7f7f9288c9bc in stfk_CopyTokenAttributes /home/user/Desktop/nss_ubsan/nss/lib/softoken/pkcs11u.c:1328
#8 0x7f7f9288d50a in sftk_CopyTokenObject /home/user/Desktop/nss_ubsan/nss/lib/softoken/pkcs11u.c:1499
#9 0x7f7f9288d7f7 in sftk_CopyObject /home/user/Desktop/nss_ubsan/nss/lib/softoken/pkcs11u.c:1551
#10 0x7f7f9281052d in NSC_CopyObject /home/user/Desktop/nss_ubsan/nss/lib/softoken/pkcs11.c:4240
#11 0x7f7f949a1579 in PK11_CopyTokenPrivKeyToSessionPrivKey /home/user/Desktop/nss_ubsan/nss/lib/pk11wrap/pk11akey.c:2004
#12 0x7f7f95b0fdb8 in SSL_ConfigSecureServerWithCertChain /home/user/Desktop/nss_ubsan/nss/lib/ssl/sslsecur.c:872
#13 0x7f7f95b0fad5 in SSL_ConfigSecureServer /home/user/Desktop/nss_ubsan/nss/lib/ssl/sslsecur.c:820
#14 0x42ec46 in server_main /home/user/Desktop/nss_ubsan/nss/cmd/selfserv/selfserv.c:1910
#15 0x43537e in main /home/user/Desktop/nss_ubsan/nss/cmd/selfserv/selfserv.c:2643
#16 0x7f7f934d5ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
#17 0x407890 in _start ??:?
Mass cc to get some NSS eyes on these bugs.
Group: core-security → crypto-core-security
Yeah, this is a valid complaint of undefined behaviour.

Under ToT ( ), this corresponds to line 1007 of lgattr.c, which is the invocation of the LG_CLONE_ATTR macro.

If CKA_LABEL is null, LG_CLONE_ATTR is called with lg_StaticNullAttr, for which lg_StaticNullAttr.pValue is NULL (see line 211)

This same bug would affect CKA_SUBJECT, CKA_START_DATE, CKA_END_DATE, and CKA_ID (for unknown keys).

Tyson, would you be able to check to see if the following patch to line 139 squelches things

if (value != NULL) {
attr->ulValueLen = len;
return CKR_OK;

resolves this?
Flags: needinfo?(twsmith)
That patch fixes the error.
Flags: needinfo?(twsmith)
In the proposed patch in comment 2, if value _is_ null should we set len to 0 so later code doesn't try to read stuff that isn't there?
Group: crypto-core-security
Flags: needinfo?(ryan.sleevi)
Keywords: crash, csectype-dos
Dan: value isn't used beyond that memcpy, and attr->pValue's nullness or non-nullness should be addressed by the len=0.

It'd only be an issue for situations where value != NULL, but len==0, but that would be a developer-error (since this functionality isn't publicly exposed), and can quickly be checked in the existing code as an unreachable situation. So I don't think anything beyond the proposed would be necessary.
Flags: needinfo?(ryan.sleevi)
Since it was confirmed already in comment 2 that the solution solves the problem I landed this as
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.25
You need to log in before you can comment on or make changes to this bug.