Closed Bug 494087 Opened 11 years ago Closed 11 years ago

Passing NULL as the value of cert_pi_trustAnchors causes a crash in cert_pkixSetParam

Categories

(NSS :: Libraries, defect, P1)

3.12
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.12.4

People

(Reporter: wtc, Assigned: alvolkov.bgs)

Details

(Whiteboard: PKIX MOZ)

Attachments

(1 file)

According to documentation of cert_pi_trustAnchors, we can
just pass NULL to use the default trusted roots:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certdb/certt.h&rev=1.50&mark=943-944#942

When the Chromium browser tried that on Ubuntu 8.04 (which uses
NSS 3.12.0.3), it resulted in a crash by SIGSEGV in cert_pkixSetParam
in mozilla/security/nss/lib/certhigh/certvfypkix.c because it
doesn't handle the NULL case as advertised.

I verified that the cert_pi_trustAnchors case in cert_pkixSetParam
hasn't changed since NSS_3_12_RTM, so you should be able to
reproduce this crash with the current NSS trunk.  I guess
it crashes here:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certhigh/certvfypkix.c&rev=1.42&mark=1698#1690

because CERT_LIST_HEAD deferences its argument:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certdb/certt.h&rev=1.50&mark=378#378
Alexei, will we get the desired behavior if we pass a pointer
to an empty CERTCertList as the value of cert_pi_trustAnchors?
Wan-Teh,
Please confirm that this behavior is still present in 3.12.3 or 3.12.4 RC0.
It may have been fixed already.
By code inspection, this bug should be still present in the current
trunk because the code in question hasn't changed since NSS_3_12_RTM:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certhigh/certvfypkix.c&rev=1.42&mark=1691-1723#1690
I verified by patching vfychain.c that this bug is still present
in the current trunk:

wtc@fips:~/nss-tip/mozilla/security/nss/cmd/vfychain/Linux2.6_x86_glibc_PTH_DBG
.OBJ$ gdb ./vfychain
GNU gdb 6.8-gg14
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux"...
FAQ: http://go/gdb, Wiki: http://wiki/Main/GnuDebugger, Email: gdb-team
(gdb) run -u 3 -p -p -a verisign.pem
Starting program: /home/wtc/nss-tip/mozilla/security/nss/cmd/vfychain/Linux2.6_x
86_glibc_PTH_DBG.OBJ/vfychain -u 3 -p -p -a verisign.pem
warning: Lowest section in system-supplied DSO at 0xffffe000 is .hash at ffffe0b
4
[Thread debugging using libthread_db enabled]
[New Thread 0xf7dbd6c0 (LWP 12195)]
Warning: guessed host libthread_db library "/usr/grte/v1/lib64/libthread_db.so.1
".
Using host libthread_db library "/usr/grte/v1/lib64/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xf7dbd6c0 (LWP 12195)]
0xf7e64cda in cert_pkixSetParam (procParams=0x8075018, param=0x8060ea0,
    plContext=0x8073d50) at certvfypkix.c:1698
1698                for(node = CERT_LIST_HEAD(certList); !CERT_LIST_END(node, ce
rtList);
(gdb) where
#0  0xf7e64cda in cert_pkixSetParam (procParams=0x8075018, param=0x8060ea0,
    plContext=0x8073d50) at certvfypkix.c:1698
#1  0xf7e6518f in CERT_PKIXVerifyCert (cert=0x8076428, usages=8,
    paramsIn=0x8060ea0, paramsOut=0x8060f40, wincx=0x8060e20)
    at certvfypkix.c:2125
#2  0x0804c7b9 in main (argc=7, argv=0xffffd8a4, envp=0xffffd8c4)
    at vfychain.c:720
(gdb) print certList
$1 = (const CERTCertList *) 0x0
(gdb)

I found that I got the desired behavior (use all default trusted
roots) if I don't specify cert_pi_trustAnchors or I pass a pointer
to an empty cert list (CERT_NewCertList()).  Perhaps all we need
to do is to fix the comment in certt.h.
Setting to P1, at least until problem analysis is complete.
Priority: -- → P1
Target Milestone: --- → 3.12.4
Whiteboard: PKIX MOZ
I added some info in bug 494489 comment 4 that can help
us document cert_pi_trustAnchors better.
(In reply to comment #1)
> Alexei, will we get the desired behavior if we pass a pointer
> to an empty CERTCertList as the value of cert_pi_trustAnchors?
You will. To make sure that libpkix uses default trusted roots you need either not to pass cert_pi_trustAnchors parameter at all or to pass an empty list.

The code definitely require to check the list argument before dereferencing the pointer. I think the correct way to fix it will be to return an error if CERTValInParam structure that passed to the function has cert_pi_trustAnchors with the chain pointing to NULL object.
Attachment #380307 - Flags: review? → review?(wtc)
Attachment #380307 - Flags: review?(wtc) → review+
Comment on attachment 380307 [details] [diff] [review]
Patch v1 - check list value before deref it(committed).

r=wtc.  This patch is not complete without updating the documentation
in certt.h:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certdb/certt.h&rev=1.50&mark=943-944#942

Please see comment 6 for the points that need to be clarified.
thanks for the review. I have changed the comment as well.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #380307 - Attachment description: Patch v1 - check list value before deref it. → Patch v1 - check list value before deref it(committed).
You need to log in before you can comment on or make changes to this bug.