1187545 - AddressSanitizer: SEGV on unknown address PL_HashTableLookupConst

Status: RESOLVED FIXED

(NSS :: Tools, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: 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

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Mass cc to get some NSS eyes on these bugs.

Comment 2

[Franziskus Kiefer [:franziskus]]

After checking a couple of certificates, this seems to happen to every certificate and also without ASAN.

Comment 3

[Franziskus Kiefer [:franziskus]]

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?

Comment 4

[Wan-Teh Chang]

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.

Comment 5

[Martin Thomson [:mt:]]

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.

Comment 6

[Franziskus Kiefer [:franziskus]]

Attachment: 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.

Comment 7

[Martin Thomson [:mt:]]

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.

Comment 8

[Wan-Teh Chang]

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

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

r=wtc.

Comment 9

[Elio Maldonado]

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.

Comment 10

[Franziskus Kiefer [:franziskus]]

No objections it seems, can you do the checkin Martin?

Comment 11

[Martin Thomson [:mt:]]

https://hg.mozilla.org/projects/nss/rev/df1729d37870