142147 - Viewing signed message crashes Mozilla, if I did not log-in into the FIPS module

Status: VERIFIED FIXED

(MailNews Core :: Security: S/MIME, defect, P2)

Description

[Steffen Motzer]

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.0+)
Gecko/20020503
BuildID:    20020503

I signed a message and tried to sent it to my e-mail address. At first, Mozilla
failed to send it, because I was not logged into the FIPS module. I wasn't asked
to log into the FIPS module by prompting for the master password. After manually
logging into the module I was finally able to send the message.

I restarted Mozilla and tried to view the message in the preview pane (without
being logged into the FIPS module), and Mozilla crashed immediately instead of
asking me for the password!


Reproducible: Always

Comment 1

[John Unruh]

S/MIME

Comment 2

[Charles Rosendahl]

Reporter,

Did you try to sign and encrypt the message, or sign only?  Also, is this win2k
or winXP?

Is this a brand new profile (never used before) and this is the very first
message being sent, or has this profile been in use for some time using s/mime
capabilities?

Thanks

Comment 3

[Steffen Motzer]

I only signed the message. I have been signing and encrypting messages (under
Windows 2000) without any problems for some time without having enabled FIPS.

After having enabled FIPS, Mozilla crashes in PK11_SetPasswordFunc whenever I
try to view a message that I signed (i.e. the copy in the "Sent" folder) and I
am not already logged into the FIPS module.

Comment 4

[Charles Rosendahl]

Confirmed.
Incident ID 6004602
Incident ID 6004526

-Enable FIPS.
-Attempt to send a signed message to yourself
The error message indicates there is something wrong with the certs (no password
prompt)
-Go to mail&news account settings->security.  click the select button.  Crash

multiple variations occur to cause the crashes above.

Comment 5

[Charles Rosendahl]

added nomination. Must fix by RTM - too late for beta, yes?

Comment 6

[Kai Engert [:KaiE:]]

-> me

Comment 7

[Charles Rosendahl]

Me believes this fixed with the latest work we've done around FIPS, yes?  I
don't experience the problems any more on the 20020528 builds.

Comment 8

[Steffen Motzer]

It's definitely not fixed. Build 20020529 still crashes and when I try to send a
signed message (and FIPS is enabled) Mozilla fails to send it and pops up the
errormessage 'sending of message failed' instead of asking for the passphrase.

Comment 9

[Charles Rosendahl]

2002053008 Trunk Build

I did get one crash, but cannot reporduce as of yet.  climate is down, so I
can't get the incident ID.  I am using the nightly builds (aka mozilla
nightly's) and usually see the "Send message failed" error dialog.

You are correct - the bug still exists wherein there is no prompt to log into
FIPS when you send the message.

Comment 10

[Kai Engert [:KaiE:]]

If this is a trunk only crash, it is not relevant for beta. Removing keywords
and status. I suspect it is trunk, because your user agent says "1.0.0+", and
Charles mentioned trunk only, too.

Please re-add the status if you are able to reproduce with a branch build.

Comment 11

[Steffen Motzer]

Unfortunately it is _not_ a trunk only problem. I just tried it with Mozilla
release 1.0 and it crashes too when I try to open a self-signed message and FIPS
is enabled (when I open a signed message that someone else signed - or if my
certificate isn't imported - then Mozilla does not crash).

Comment 12

[bsharma]

It does happen on the branch builds and the trunk builds.  I have seen it
recently on both using the 0605 and 0604 builds.

Comment 13

[Charles Rosendahl]

Sorry - comment 12 was me.

Comment 14

[Kai Engert [:KaiE:]]

Ok, I can now reproduce it reliably on the branch, too.
Adding back the keywords that I removed yesterday.

Comment 15

[Kai Engert [:KaiE:]]

To reproduce I use a configuration, where opening the S/Mime message is the
security/crypto operation in the browser session.

I prepared my environment to have a copy of a signed S/Mime message in my local
folders. FIPS is turned on.

While doing everytime the same thing, I have seen different reasons for
crashing, segmentation faults, complaints about trying to execute an illegal
instruction, and once I saw the debugger to be so confused, that it was unable
to provide a stack track.

This makes me believe that we see memory/heap corruption.

I'm attaching a stack trace. When I got a stack trace, I looked like that. In
the attach trace, it stops at GetProxyForObject, called by dereferencing a
pointer. The pointer is not null, the value looks like it could be a valid pointer.

Once I saw the execution stoped inside one level below, in the middle of
executing GetProxyForObject - but that didn't reveal any additional hints
either. No null pointers involved.

(I noticed that the first parameter to GetProxyForOjbect is a pointer, which
points to (void*)1. But that's intended, it's a special value defined as
NS_UI_THREAD_EVENTQ, and I believe that pointer will not be dereferenced.)

I think this is a NSS bug.

Comment 16

[Kai Engert [:KaiE:]]

Attachment: Crash Stack Trace

Comment 17

[Charles Rosendahl]

*** Bug 150673 has been marked as a duplicate of this bug. ***

Comment 18

[Charles Rosendahl]

see stack traces 6390095, 7199040.  After restarting and attempting to view the
message see stack trace 7199086.

Comment 19

[Charles Rosendahl]

correction - 6390095 should be 7197186

Comment 20

[selmer (gone)]

Can we get an ETA on the fix?

Comment 21

[Kai Engert [:KaiE:]]

Re ETA: This bug is assigned to me, but I don't have an idea what I could do to
solve the problem. The talkbalk reports mentioned do not give additional
information over what I already reported.

Comment 22

[Stephane Saux]

adt2.  This is a crash, and we'd like to fix it, but it seems really hard to
track. Moreover, this won't affect a large number of people.

Comment 23

[Kai Engert [:KaiE:]]

Ok, this was tricky, but I found the cause.

The CMS decoder creates a NSS data structure. When it did so, it passed in a
pointer to a context object - that is required when NSS decides that it needs to
bring up a password prompt.

When the decoding is finished, the CMS decoder creates a new wrapper object
instance for the lower NSS data type.

What happens here is:
- the lower level NSS object still carries a pointer to the context
- the decoder object goes out of scope before the password prompt comes up
- the decoder's destructor destroys the context object, as it is the only one
which carries a reference to it
- the password prompt access destroyed memory and crashes

The solution is to make the new wrapper object carry a reference to the context,
too.

Comment 24

[Kai Engert [:KaiE:]]

Attachment: Patch v1

Fix as described in previous comment.

Comment 25

[Kai Engert [:KaiE:]]

Javi, can you please review?

Comment 26

[Javier Delgadillo]

Comment on attachment 95567 [details] [diff] [review]
Patch v1

r=javi

Comment 27

[Brian Ryner (not reading)]

Comment on attachment 95567 [details] [diff] [review]
Patch v1

>Index: mozilla/security/manager/ssl/src/nsCMS.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/security/manager/ssl/src/nsCMS.cpp,v
>retrieving revision 1.6
>diff -u -r1.6 nsCMS.cpp
>--- mozilla/security/manager/ssl/src/nsCMS.cpp	24 Apr 2002 00:40:49 -0000	1.6
>+++ mozilla/security/manager/ssl/src/nsCMS.cpp	16 Aug 2002 12:52:01 -0000
>@@ -578,7 +578,12 @@
>   NSSCMSMessage *cmsMsg;
>   cmsMsg = NSS_CMSDecoder_Finish(m_dcx);
>   if (cmsMsg) {
>-    nsCOMPtr<nsICMSMessage> msg = new nsCMSMessage(cmsMsg);
>+    nsCMSMessage *obj = new nsCMSMessage(cmsMsg);
>+    // The NSS object cmsMsg still carries a reference to the context
>+    // we gave it on construction.
>+    // Make sure the context will live long enough.
>+    obj->referenceContext(m_ctx);
>+    nsCOMPtr<nsICMSMessage> msg = obj;
>     *aCMSMsg = msg;
>     NS_ADDREF(*aCMSMsg);
>   }

Why bother with the nsCOMPtr at all?  Just assign *aCMSMsg = obj.

Comment 28

[Kai Engert [:KaiE:]]

Attachment: Patch v2

Agreed

Comment 29

[Kai Engert [:KaiE:]]

Comment on attachment 95858 [details] [diff] [review]
Patch v2

carrying forward r=javi

Comment 30

[Brian Ryner (not reading)]

Comment on attachment 95858 [details] [diff] [review]
Patch v2

sr=bryner

Comment 31

[Kai Engert [:KaiE:]]

fixed on trunk

Comment 32

[Charles Rosendahl]

This bug has been verified.  There are no longer crashes when attempting to view
a message while not logged into FIPS.