Closed Bug 494087 Opened 12 years ago Closed 12 years ago
Passing NULL as the value of cert
_pi _trust Anchors causes a crash in cert _pkix Set Param
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 126.96.36.199), 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
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.
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: 12 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.