524167 - Crash in [@ find_objects_by_template - nssToken_FindCertificateByIssuerAndSerialNumber]

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Marcia Knous [:marcia]]

Seen while running  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20091023 Minefield/3.7a1pre

STR:
1. Not sure, crashed after doing a draft message in Gmail.

Crash ID: http://crash-stats.mozilla.com/report/index/bp-85e50e16-9428-4a1c-9c2b-a8b792091023

I also crashed in the same stack on the 20th.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Assuming the stack in the crash dump is correct, the crash happened at 
http://hg.mozilla.org/mozilla-central/annotate/69a4a21d9b84/security/nss/lib/dev/devtoken.c#l447
I'd guess that the "token" argument is NULL.
It appears that this argument value gets passed all the way down the stack
through quite a few functions that never check it for NULL before it gets 
here.  This is the first place it is used.  

PK11_ImportCert gets a token pointer by calling 
   850    NSSToken *token = PK11Slot_GetNSSToken(slot);
but never checks it for NULL, and passes it down the stack to the point 
where it crashes.  That's my hypothesis.

Marcia,
Is this using FIPS mode?

Comment 2

[Marcia Knous [:marcia]]

I may have previously used FIPS mode in this profile, but at the time of the crash FIPS was not enabled.

Comment 3

[timeless]

Signature	find_objects_by_template
UUID	85e50e16-9428-4a1c-9c2b-a8b792091023
Time 	2009-10-23 11:00:07.576207
Uptime	3651
Last Crash	178084 seconds before submission
Product	Firefox
Version	3.7a1pre
Build ID	20091023060247
Branch	1.9.3
OS	Mac OS X
OS Version	10.6.1 10B504
CPU	x86
CPU Info	GenuineIntel family 6 model 14 stepping 8
Crash Reason	EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE
Crash Address	0x30
User Comments	
Processor Notes 	
Related Bugs

Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	libnss3.dylib 	find_objects_by_template 	devtoken.c:447
1 	libnss3.dylib 	nssToken_FindCertificateByIssuerAndSerialNumber 	devtoken.c:864
2 	libnss3.dylib 	nssToken_ImportCertificate 	devtoken.c:520
3 	libnss3.dylib 	PK11_ImportCert 	pk11cert.c:911
4 	XUL 	AuthCertificateCallback 	security/manager/ssl/src/nsNSSCallbacks.cpp:1020
5 	libssl3.dylib 	ssl3_HandleHandshakeMessage 	ssl3con.c:7280
6 	libssl3.dylib 	ssl3_HandleRecord 	ssl3con.c:8083
7 	libssl3.dylib 	ssl3_GatherCompleteHandshake 	ssl3gthr.c:206
8 	libssl3.dylib 	ssl_GatherRecord1stHandshake 	sslcon.c:1258
9 	libssl3.dylib 	ssl_Do1stHandshake 	sslsecur.c:151
10 	libssl3.dylib 	ssl_SecureSend 	sslsecur.c:1176
11 	libssl3.dylib 	ssl_SecureWrite 	sslsecur.c:1221
12 	libssl3.dylib 	ssl_Write 	sslsock.c:1488
13 	XUL 	nsSSLThread::Run 	security/manager/ssl/src/nsSSLThread.cpp:1045
14 	libnspr4.dylib 	_pt_root 	nsprpub/pr/src/pthreads/ptthread.c:228
15 	libSystem.B.dylib 	_pthread_start 	
16 	libSystem.B.dylib 	thread_start

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Attachment: Patch v1 - Untested

The info that Timeless added confirms that the token pointer is NULL.
This patch adds sanity checks for NULL token pointers to all the relevant
functions.  

This patch builds, so I will ask Bob to review it.  I will try to test it 
tonight.

Comment 5

[timeless]

note that i just grabbed it from comment 0, but indeed, this is why it's a good idea to inline the basic bit + crashing frame stack (up to 20 frames). glad to be of service.

Comment 6

[Robert Relyea]

Comment on attachment 410091 [details] [diff] [review]
Patch v1 - Untested

r+=rrelyea

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Bug 503418 is marked as blocking but requires this patch. Asking for blocking1.9.2

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

What's the distribution vector here? Does this imply a new NSS tag?

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

This bug fix is targeted for NSS 3.12.6, which will follow NSS 3.12.5,
which is now in final release stages (I believe).

Comment 10

[chris hofmann]

I see only 0-3 crashes per day across all firefox releases over the last two months on this signature.   might be a good candidate for a security/stability release if the fix involves updating to a new nss with other changes.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

This was only marked as a blocker due to the dependency from bug 503418, but I no longer think that blocks Firefox 3.6 (would rather release note it) and feel like we should address these issues when NSS 3.12.6 comes available.

People know what to do if they disagree, but basically, I don't think the problem in bug 503418 (unable to toggle FIPS mode without crashing, and occasional crashes in FIPS mode warrants it; also, there's indications in bug 503418 comment 47 indicates it isn't an issue anymore.

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

dev/devtoken.c;      new revision: 1.52;  previous revision: 1.51
pk11wrap/pk11cert.c; new revision: 1.172; previous revision: 1.171

Comment 14

[Marcia Knous [:marcia]]

This crash persists in Firefox 3.6 beta 5 - Did NSS 3.12.6 make it into this release? I am having difficult toggling FIPS - Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIPKCS11ModuleDB.toggleFIPSMode]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://pippki/content/device_manager.js :: toggleFIPS :: line 545"  data: no] appears in the console.

http://crash-stats.mozilla.com/report/index/bp-39ec4691-4ac7-46e1-96d2-da8502091218 is my report.

Comment 15

[Nelson Bolyard (seldom reads bugmail)]

NSS 3.12.6 is not yet released.  It's waiting for a fix for the SSL/TLS renegotiation bug, and for server side SNI, both now in development, 
expected soon.