621664 - TB crashes in non-crypto code after attempt to generate signed msg fails

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Matej Kurpel]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; sk-SK; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.13) Gecko/20101207 Thunderbird/3.1.7

I am developing my own PKCS#11 module to sign my e-mail. Everything works when C_SignInit returns CKR_OK. But when it returns CKR_FUNCTION_CANCELED, Thunderbird crashes. Tried other CKR_ return codes such ask CKR_FUNCTION_NOT_SUPPORTED, the crash remains. (this is to implement authentication on the side of the device - when it fails, the module returns CKR_FUNCTION_CANCELED). I can see C_CloseSession being called after C_SignInit (see the pkcs11-spy log).

Reproducible: Always

Steps to Reproduce:
1. Load a PKCS#11 module which returns CKR_FUNCTION_CANCELED in C_SignInit
2. Set up your digital signature properties as usual
3. Compose a new message, choose Security - Digitally sign this message
4. Send your e-mail and see the crash.
Actual Results:  
Sometimes Visual C++ Runtime Error, sometimes a black little window behind the window with progress bar, sometimes Mozilla Crash Reporter dialog. See the screenshot: http://img4.glowfoto.com/images/2010/12/10-1150202661L.png


Expected Results:  
Display an error message box that the message cannot be sent.

-------- pkcs11-spy log with unneeded information omitted ------

42: C_GetAttributeValue
[in] hSession = 0x1
[in] hObject = 0x2
[in] pTemplate[1]: 
    CKA_PRIVATE           requested with 1 buffer
[out] pTemplate[1]: 
    CKA_PRIVATE           True
Returned:  0 CKR_OK


43: C_OpenSession
[in] slotID = 0x0
[in] flags = 0x4
pApplication=068E8400
Notify=61A6A378
[out] *phSession = 0x2
Returned:  0 CKR_OK


44: C_SignInit
[in] hSession = 0x2
pMechanism->type=CKM_RSA_PKCS                 
[in] hKey = 0x2
Returned:  80 CKR_FUNCTION_CANCELED


45: C_CloseSession
[in] hSession = 0x2
Returned:  0 CKR_OK

-------------- pks11-spy log end (TB crashed here) -----------

----- call stack begin -----

>    thunderbird.exe!nsGlobalWindow::SetNewDocument(nsIDocument * aDocument=0x00a02c00, nsISupports * aState=0x00000000, int aClearScopeHint=0x00000001, int aIsInternalCall=0x0000000b)  Line 1760 + 0x3 bytes    C++
     thunderbird.exe!nsGlobalWindow::SetNewDocument(nsIDocument * aDocument=0x00a02c00, nsISupports * aState=0x00000000, int aClearScopeHint=0x00000001)  Line 1569    C++
     thunderbird.exe!DocumentViewerImpl::InitInternal(nsIWidget * aParentWidget=0x04e498c0, nsISupports * aState=0x00000000, const nsIntRect & aBounds={...}, int aDoCreation=0x00000001, int aInPrintPreview=0x00000000, int aNeedMakeCX=0x00000001)  Line 960    C++
     thunderbird.exe!DocumentViewerImpl::Init(nsIWidget * aParentWidget=0x00a79580, const nsIntRect & aBounds={...})  Line 699    C++
     thunderbird.exe!nsDocShell::SetupNewViewer(nsIContentViewer * aNewViewer=0x04e8c3c0)  Line 7304 + 0x1b bytes    C++
     thunderbird.exe!nsDocShell::Embed(nsIContentViewer * aContentViewer=0x04e8c3c0, const char * aCommand=0x01ab0481, nsISupports * aExtraInfo=0x00000000)  Line 5472    C++
     thunderbird.exe!nsDocShell::CreateContentViewer(const char * aContentType=0x03c37d68, nsIRequest * request=0x050c6740, nsIStreamListener * * aContentHandler=0x050c6740)  Line 7090 + 0x15 bytes    C++
     thunderbird.exe!nsDSURIContentListener::DoContent(const char * aContentType=0x03c37d68, int aIsContentPreferred=0x00000000, nsIRequest * request=0x050c6740, nsIStreamListener * * aContentHandler=0x04effb5c, int * aAbortProcess=0x0045ac48)  Line 150    C++
     thunderbird.exe!nsDocumentOpenInfo::TryContentListener(nsIURIContentListener * aListener=0x06eb4e80, nsIChannel * aChannel=0x04effb5c)  Line 734    C++
     thunderbird.exe!nsDocumentOpenInfo::DispatchContent(nsIRequest * request=0x050c6740, nsISupports * aCtxt=0x00000000)  Line 434 + 0x15 bytes    C++
     thunderbird.exe!nsDocumentOpenInfo::OnStartRequest(nsIRequest * request=0x050c6740, nsISupports * aCtxt=0x00000000)  Line 287    C++
     thunderbird.exe!nsJARChannel::OnStartRequest(nsIRequest * req=0x05bac330, nsISupports * ctx=0x00000000)  Line 867 + 0x16 bytes    C++
     thunderbird.exe!nsInputStreamPump::OnStateStart()  Line 445    C++
     thunderbird.exe!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x04e7cb68)  Line 407    C++
     xpcom_core.dll!nsOutputStreamReadyEvent::Run()  Line 113    C++
     xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0045aef0)  Line 527 + 0x6 bytes    C++
     xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00000001, int mayWait=0x00000001)  Line 250 + 0xd bytes    C++
     xpcom_core.dll!nsThread::Shutdown()  Line 468 + 0xa bytes    C++
     thunderbird.exe!nsSound::PurgeLastSound()  Line 140    C++
     thunderbird.exe!nsSound::~nsSound()  Line 135    C++
     thunderbird.exe!nsSound::`scalar deleting destructor'()  + 0x8 bytes    C++
     thunderbird.exe!nsIndexedToHTML::Release()  Line 62 + 0x18 bytes    C++
     thunderbird.exe!XPCJSRuntime::GCCallback(JSContext * cx=0x04f1d400, JSGCStatus status=JSGC_END)  Line 760 + 0x2a bytes    C++
     thunderbird.exe!DOMGCCallback(JSContext * cx=0x04f1d400, JSGCStatus status=JSGC_END)  Line 3827 + 0x14 bytes    C++
     thunderbird.exe!XPCCycleCollectGCCallback(JSContext * cx=0x04f1d400, JSGCStatus status=JSGC_END)  Line 412 + 0x10 bytes    C++
     js3250.dll!js_GC(JSContext * cx=0x04f1d400, JSGCInvocationKind gckind=GC_NORMAL)  Line 3822 + 0x5 bytes    C++
     js3250.dll!JS_GC(JSContext * cx=0x04f1d400)  Line 2439 + 0x8 bytes    C++
     thunderbird.exe!nsXPConnect::Collect()  Line 479    C++
     xpcom_core.dll!nsCycleCollector::Collect(unsigned int aTryCollections=0x00000001)  Line 2434 + 0x5 bytes    C++
     xpcom_core.dll!nsCycleCollector_collect()  Line 3130    C++
     thunderbird.exe!nsJSContext::CC()  Line 3641 + 0x6 bytes    C++
     thunderbird.exe!nsJSContext::IntervalCC()  Line 3730    C++
     xpcom_core.dll!nsTimerImpl::Fire()  Line 427 + 0x6 bytes    C++
     xpcom_core.dll!nsTimerEvent::Run()  Line 521    C++
     xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0045f060)  Line 527 + 0x6 bytes    C++
     xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00000001, int mayWait=0x00000001)  Line 250 + 0xd bytes    C++
     thunderbird.exe!nsXULWindow::ShowModal()  Line 416 + 0x9 bytes    C++
     thunderbird.exe!nsContentTreeOwner::ShowAsModal()  Line 529    C++
     thunderbird.exe!nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow * aParent=0x00a47780, const char * aUrl=0x01c5e844, const char * aName=0x01ab0bd0, const char * aFeatures=0x01ab0bd8, int aDialog=0x00000001, nsIArray * argv=0x04e20720, int aCalledFromJS=0x00000000, nsIDOMWindow * * _retval=0x0045f35c)  Line 1011    C++
     thunderbird.exe!nsWindowWatcher::OpenWindow(nsIDOMWindow * aParent=0x00a47780, const char * aUrl=0x01c5e844, const char * aName=0x01ab0bd0, const char * aFeatures=0x01ab0bd8, nsISupports * aArguments=0x04eef740, nsIDOMWindow * * _retval=0x0045f35c)  Line 425 + 0x24 bytes    C++
     thunderbird.exe!nsPromptService::DoDialog(nsIDOMWindow * aParent=0x04e498c0, nsIDialogParamBlock * aParamBlock=0x04eef740, const char * aChromeURL=0x01c5e844)  Line 797    C++
     thunderbird.exe!nsPromptService::Alert(nsIDOMWindow * parent=0x00a47780, const wchar_t * dialogTitle=0x04eff8e0, const wchar_t * text=0x0773c268)  Line 148    C++
     thunderbird.exe!nsPrompt::Alert(const wchar_t * dialogTitle=0x04eff8e0, const wchar_t * text=0x0773c268)  Line 199    C++
     thunderbird.exe!nsMsgDisplayMessageByString(nsIPrompt * aPrompt=0x06eb4e40, const wchar_t * msg=0x0773c268, const wchar_t * windowTitle=0x04eff8e0)  Line 124    C++
     thunderbird.exe!nsMsgSendReport::DisplayReport(nsIPrompt * prompt=0x06eb4e40, int showErrorOnly=0x00000001, int dontShowReportTwice=0x00000000, unsigned int * _retval=0x0045f57c)  Line 428 + 0xe bytes    C++
     thunderbird.exe!nsMsgComposeAndSend::Fail(unsigned int failure_code=0x06eb4e40, const wchar_t * error_msg=0x00000000, unsigned int * _retval=0x0045f57c)  Line 3812    C++
     thunderbird.exe!nsMsgComposeAndSend::GatherMimeAttachments()  Line 1147    C++
     thunderbird.exe!nsMsgAttachmentHandler::UrlExit(unsigned int status=0x00000000, const wchar_t * aMsg=0x00000000)  Line 1315 + 0x4 bytes    C++
     thunderbird.exe!FetcherURLDoneCallback(unsigned int aStatus=0x00000000, const nsACString_internal & aContentType={...}, const nsACString_internal & aCharset={...}, int totalSize=0x000000f1, const wchar_t * aMsg=0x00000000, void * tagData=0x04e56920)  Line 534 + 0xd bytes    C++
     thunderbird.exe!nsURLFetcher::OnStopRequest(nsIRequest * request=0x04e7ce0c, nsISupports * ctxt=0x00000000, unsigned int aStatus=0x00000000)  Line 327 + 0x15 bytes    C++
     thunderbird.exe!nsDocumentOpenInfo::OnStopRequest(nsIRequest * request=0x04e7ce0c, nsISupports * aCtxt=0x00000000, unsigned int aStatus=0x00000000)  Line 324    C++
     thunderbird.exe!nsBaseChannel::OnStopRequest(nsIRequest * request=0x04e63b00, nsISupports * ctxt=0x00000000, unsigned int status=0x00000000)  Line 681    C++
     thunderbird.exe!nsInputStreamPump::OnStateStop()  Line 579    C++
     thunderbird.exe!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x04e7d428)  Line 404    C++
     xpcom_core.dll!nsOutputStreamReadyEvent::Run()  Line 113    C++
     xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0045f8dc)  Line 527 + 0x6 bytes    C++
     xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00000001, int mayWait=0x00000001)  Line 250 + 0xd bytes    C++
     thunderbird.exe!nsBaseAppShell::Run()  Line 177 + 0x9 bytes    C++
     thunderbird.exe!nsAppStartup::Run()  Line 184    C++
     thunderbird.exe!XRE_main(int argc=0x00000001, char * * argv=0x00a110a8, const nsXREAppData * aAppData=0x00a17340)  Line 3485    C++
     thunderbird.exe!NS_internal_main(int argc=0x00000001, char * * argv=0x00a110a8)  Line 102    C++
     thunderbird.exe!wmain(int argc=0x00a110a8, wchar_t * * argv=0x00a1b700)  Line 122    C++
     thunderbird.exe!__tmainCRTStartup()  Line 591 + 0x19 bytes    C
     kernel32.dll!@BaseThreadInitThunk@12()  + 0x12 bytes
     ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes
     ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes 

------------ call stack end --------------

crashed on a line in nsGlobalWindow.cpp:

nsWindowSH::InvalidateGlobalScopePolluter(cx, currentInner->mJSObject);

saying Uncaught exception occurred.

(debugged using VS2008)

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

This bug is NOT about why the attempt to generate the signature failed.
It is about the fact that TB crashes, WAY outside of SMIME code, when it 
cannot generate a signed message. That's not a NSS or PSM fault.  

The stack shows that nsMsgComposeAndSend::GatherMimeAttachments fails, calling nsMsgComposeAndSend::Fail which then tries to put up a modal alert dialog
(nsPromptService::Alert calls nsPromptService::DoDialog calls 
nsXULWindow::ShowModal).  

Then ProcessNextEvent runs the GC on the stack (?!) which destroys a sound object, which tries to shutdown a thread (This may be a red herring).  nsSound::~nsSound calls thunderbird.exe!nsSound::PurgeLastSound() which calls nsThread::Shutdown().

Then ProcessNextEvent calls nsDSURIContentListener::DoContent calls nsDocShell::CreateContentViewer, which dies trying to init DocumentViewerImpl::Init.  Some NULL pointers on the stack are suspicious.

Comment 2

[David :Bienvenu]

Perhaps bz might have some suggestions about what's going on...

Comment 3

[Boris Zbarsky [:bzbarsky]]

What's actually causing the crash?  That is, which thing in SetNewDocument is not kosher?

Comment 4

[David :Bienvenu]

(In reply to comment #3)
> What's actually causing the crash?  That is, which thing in SetNewDocument is
> not kosher?

I haven't reproduced this so I don't know what's not kosher.

Comment 5

[Wayne Mery (:wsmwk)]

Matej, can you supply the code to David?

Comment 6

[Matej Kurpel]

The code of my PKCS#11 module? I could do that but I am afraid it couldn't be sooner than June 2011 (that's when I do my final exams and defend my diploma thesis which consists of the PKCS#11 module too).

Comment 7

[David Cooper]

I have encountered the same problem of Thunderbird crashing when an attempt to digitally sign a message fails.  I don't know if the crash is occurring for me in the same place as it is for Matej, but I have verified that the problem is in the SMIME code of NSS and I have a fix for it.

In my case, the signature is failing when using a PIV Card with OpenSC 0.12.0.  Doug Engert has created a patch (https://bugzilla.mozilla.org/show_bug.cgi?id=613507) that partially addresses the problem of the signature operation failing in the case of PIV Cards when using OpenSC, but that patch doesn't address the problem of Thunderbird crashing when signing does fail.

The problem is in the function NSS_CMSSignerInfo_Sign in mozilla/security/nss/lib/smime/cmssiginfo.c.  After SEC_SignData is called on line 272 there is a call to PORT_FreeArena(tmppoolp, PR_FALSE) on line 274.  The "if" statement on line 281 then causes a jump to "loser" where PORT_FreeArena(tmppoolp, PR_FALSE) is called again on line 302 since tmppoolp is not NULL.

I was able to fix this problem by simply adding a line "tmppoolp = NULL;" immediately after the call to PORT_FreeArena(tmppoolp, PR_FALSE) on line 274.

Comment 8

[Ludovic Hirlimann [:Usul]]

(In reply to comment #7)
 
> The problem is in the function NSS_CMSSignerInfo_Sign in
> mozilla/security/nss/lib/smime/cmssiginfo.c.  After SEC_SignData is called on
> line 272 there is a call to PORT_FreeArena(tmppoolp, PR_FALSE) on line 274. 
> The "if" statement on line 281 then causes a jump to "loser" where
> PORT_FreeArena(tmppoolp, PR_FALSE) is called again on line 302 since tmppoolp
> is not NULL.
> 
> I was able to fix this problem by simply adding a line "tmppoolp = NULL;"
> immediately after the call to PORT_FreeArena(tmppoolp, PR_FALSE) on line 274.

Could you make a patch out of this and upload it here so we could start a review process ?

Comment 9

[David Cooper]

Attachment: Fix double free. Checked into NSS CVS repo.

Here is the patch to prevent NSS from crashing as a result of a double-free error after a digital signature operation fails.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 510311 [details] [diff] [review]
Fix double free.  Checked into NSS CVS repo.

Bug 621664: double free in NSS_CMSSignerInfo_Sign
Patch contributed by  David Cooper <[e-mail]>, r=nelson

Checking in cmssiginfo.c; new revision: 1.34; previous revision: 1.33

The remaining question is: is this the cause of the crash originally 
reported in this bug?

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

I'm going to take this back as an NSS bug until we know if the NSS patch is 
a complete solution or not, because I think it is likely to be so.

Comment 12

[Wayne Mery (:wsmwk)]

someone please add top of stack to summary

Comment 14

[Wan-Teh Chang]

I marked this bug FIXED because the NSS 3.13 release is imminent.

Whoever checks whether the NSS patch fixes the Thunderbird crash
originally reported in this bug, please mark the bug VERIFIED or
reopen it.

Nit: the actual checkin uses 0 instead of NULL to represent the
null pointer.

Index: mozilla/security/nss/lib/smime/cmssiginfo.c
===================================================================
RCS file: /cvsroot/mozilla/security/nss/lib/smime/cmssiginfo.c,v
retrieving revision 1.33
retrieving revision 1.34
diff -u -u -r1.33 -r1.34
--- mozilla/security/nss/lib/smime/cmssiginfo.c	28 Aug 2010 18:09:09 -0000	1.33
+++ mozilla/security/nss/lib/smime/cmssiginfo.c	7 Feb 2011 18:32:19 -0000	1.34
@@ -272,6 +274,7 @@
 	rv = SEC_SignData(&signature, encoded_attrs.data, encoded_attrs.len, 
 	                  privkey, signAlgTag);
 	PORT_FreeArena(tmppoolp, PR_FALSE); /* awkward memory management :-( */
+	tmppoolp = 0;
     } else {
 	rv = SGN_Digest(privkey, digestalgtag, &signature, digest);
     }