AddressSanitizer: SEGV on unknown address PL_HashTableLookupConst

RESOLVED FIXED in 3.22

Status

NSS
Tools
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tsmith, Assigned: fkiefer)

Tracking

(Blocks: 1 bug, {crash, csectype-nullptr})

trunk
3.22
crash, csectype-nullptr

Firefox Tracking Flags

(firefox42 affected)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Created attachment 8638858 [details]
test_case

This seems to happen with any DER with checkcert on a 64bit ASan build.

==43874==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7fc9918f0ce3 sp 0x7ffc3269ba50 bp 0x7ffc3269bab0 T0)
    #0 0x7fc9918f0ce2 in PL_HashTableLookupConst /home/user/code/nspr/Linux3.16_x86_64_glibc_PTH_64_OPT.OBJ/lib/ds/../../../lib/ds/plhash.c:349
    #1 0x7fc991db4fb5 in SECOID_FindOID_Util /home/user/code/nss/lib/util/secoid.c:2007
    #2 0x4901a7 in SECU_PrintObjectID /home/user/code/nss/cmd/lib/secutil.c:1101
    #3 0x497276 in SECU_PrintAlgorithmID /home/user/code/nss/cmd/lib/secutil.c:1319
    #4 0x4aa94d in SECU_PrintCertificate /home/user/code/nss/cmd/lib/secutil.c:2394
    #5 0x4b5524 in secu_PrintSignedDataSigOpt /home/user/code/nss/cmd/lib/secutil.c:3154
    #6 0x481472 in main /home/user/code/nss/cmd/checkcert/checkcert.c:393
    #7 0x7fc990d99ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
    #8 0x47fe4c in _start (/home/user/Desktop/afl-checkcert/checkcert+0x47fe4c)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/user/code/nspr/Linux3.16_x86_64_glibc_PTH_64_OPT.OBJ/lib/ds/../../../lib/ds/plhash.c:349 PL_HashTableLookupConst
==43874==ABORTING
Mass cc to get some NSS eyes on these bugs.
After checking a couple of certificates, this seems to happen to every certificate and also without ASAN.
Ok, so this seems to be an initialisation problem, i.e. NSS is never initialised. However, it seems no one is actually using this (otherwise someone should have noticed). Can we remove checkcert?
Flags: needinfo?(wtc)
Flags: needinfo?(martin.thomson)

Comment 4

2 years ago
Hi Franziskus,

It looks like even though we have been building checkcert, nobody has
used it for a long time because it is missing the NSS initialization
call as you noted.

We have several options.

1. checkcert can be a useful example of using some NSS functions.
When viewed this way, it makes sense to add a NSS_NoDBInit() call
and see if that fixes the crash.

2. We can simply remove checkcert from the NSS source tree. Please
check with the Red Hat NSS engineers if you want to do this.

3. We can try to understand what checkcert does and see if it is
subsumed by another NSS command. I suspect the |pp| or |vfychain|
command probably does everything |checkcert| does. If so, we can
safely remove checkcert from the NSS source tree.
Flags: needinfo?(wtc)
I'd advocate for option 2.  If it crashes, then it's hardly a good sample.  And I don't see any value in expending the effort required to get it into a usable state.
Flags: needinfo?(martin.thomson)
Created attachment 8684838 [details] [diff] [review]
remove checkcert command

patch to remove checkcert command.

Since no objections were raised (here or the mailing list), I'd like to remove checkcert. If I find time I'll reuse some of the code in external examples. But we shouldn't have code like this in the tree.
Assignee: nobody → franziskuskiefer
Attachment #8684838 - Flags: review?(wtc)
Attachment #8684838 - Flags: review?(martin.thomson)
Comment on attachment 8684838 [details] [diff] [review]
remove checkcert command

Review of attachment 8684838 [details] [diff] [review]:
-----------------------------------------------------------------

I'm OK with this is Elio is.
Attachment #8684838 - Flags: review?(martin.thomson)
Attachment #8684838 - Flags: review?(emaldona)
Attachment #8684838 - Flags: review+

Comment 8

2 years ago
Comment on attachment 8684838 [details] [diff] [review]
remove checkcert command

Review of attachment 8684838 [details] [diff] [review]:
-----------------------------------------------------------------

r=wtc.
Attachment #8684838 - Flags: review?(wtc) → review+

Comment 9

2 years ago
Comment on attachment 8684838 [details] [diff] [review]
remove checkcert command

Review of attachment 8684838 [details] [diff] [review]:
-----------------------------------------------------------------

I'm okay as well, checkcert is not currently being used by anything else.
Attachment #8684838 - Flags: review?(emaldona) → review+
No objections it seems, can you do the checkin Martin?
Flags: needinfo?(martin.thomson)
https://hg.mozilla.org/projects/nss/rev/df1729d37870
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(martin.thomson)
Resolution: --- → FIXED
Target Milestone: --- → 3.22
You need to log in before you can comment on or make changes to this bug.