353558 - Crash when opening encrypted draft message that requires master password [@ NSS_CMSEnvelopedData_Decode_BeforeData]

Status: RESOLVED DUPLICATE of bug 368325

(Core :: Security: PSM, defect)

Description

[Kai Engert (:KaiE:)]

I get this crash with TRUNK only. (1.8 branch works fine for me)

- Prepare your profile for sending signed/encrypted messages, get yourself a personal cert, and get a valid cert of another person, so you can send encrypted email.

- make sure you use a master password

- configure your mail account to save drafts in your local folder, so you won't need to do any logins, before getting to the drafts folder

- configure your mail account to NOT check for new messages at startup.

- Compose a mail message that is encrypted, not signed.
- Save that message as a draft.
- quit the mail application
- start the mail application
- go to the drafts folder
- click on that encrypted draft message
- you'll be prompted to enter your master password, enter it
- now you crash

Expected behaviour:
no crash


I'm crashing regardless whether OCSP is enabled or disabled.

The stack says, we are crashing in NSS.
But the NSS version used on the trunk includes very few additional patches from what is used on the 1.8 branch, so it seems more likely this is caused by some memory corruption

Note my test was on x86_64.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: crash stack

Comment 2

[Kai Engert (:KaiE:)]

(gdb) up 5
#5  0x00002aaaaeea2021 in NSS_CMSEnvelopedData_Decode_BeforeData (envd=0x1cc0e78) at cmsenvdata.c:362
362         ri = envd->recipientInfos[recipient->riIndex];
Current language:  auto; currently c
(gdb) l
357         if (!recipient->cert || !recipient->privkey) {
358             /* XXX should set an error code ?!? */
359             goto loser;
360         }
361         /* get a pointer to "our" recipientinfo */
362         ri = envd->recipientInfos[recipient->riIndex];
363
364         cinfo = &(envd->contentInfo);
365         bulkalgtag = NSS_CMSContentInfo_GetContentEncAlgTag(cinfo);
366         if (bulkalgtag == SEC_OID_UNKNOWN) {
(gdb) print recipient
$1 = (NSSCMSRecipient *) 0x1c8fce0
(gdb) print envd
$2 = (NSSCMSEnvelopedData *) 0x1cc0e78
(gdb) print envd->recipientInfos
$3 = (NSSCMSRecipientInfo **) 0x2d6f7369dada0000
(gdb) print recipient->riIndex
$4 = 0
(gdb) print envd->recipientInfos[recipient->riIndex]
Cannot access memory at address 0x2d6f7369dada0000

Comment 3

[Kai Engert (:KaiE:)]

To my surprise I do not see this crash when having the patch from bug 353597 applied.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

So, according to comment 2, at the time of the crash, the value of 
envd->recipientInfos is an invalid address.  But just a few lines above, 
in the same function, that same pointer value was used to construct a 
recipient list.  

335     /* look if one of OUR cert's issuerSN is on the list of recipients, 
336     /* get the cert and private key for it right away */
337     recipient_list = nss_cms_recipient_list_create(envd->recipientInfos);
338     if (recipient_list == NULL)
339         goto loser;

If the value of that pointer had been invalid at the time when this function
passed it to nss_cms_recipient_list_create, we would have crashed in nss_cms_recipient_list_create.  But we didn't crash there.  

So, something has corrupted the value of that pointer within the lifetime of 
this present call to function nss_cms_recipient_list_create and/or one of its 
subordinates.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Oops.  copy-n-paste error in that last sentence.  It should have read:

So, something has corrupted the value of that pointer within the lifetime of 
this present call to function NSS_CMSEnvelopedData_Decode_BeforeData and/or 
one of its subordinates.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

There has been only one change to any of the relevant CMS code in the last year,
AFAIK, and that is this change, which was intended to plug a memory leak 
reported by coverity.  See bug 341120.

<http://bonsai.mozilla.org/cvsview2.cgi?command=DIFF&subdir=mozilla%2Fsecurity%2Fnss%2Flib%2Fsmime&file=cmsreclist.c&rev1=1.4&rev2=1.5&whitespace_mode=ignore&diff_mode=full>

I reviewed and approved that patch.  If anyone can see anything wrong with it,
please tell us in this bug.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

See also bug 368325, which is virtually identical to this bug, except that
it occurs when reading a received encrypted email, rather than when reading
an encrypted email in the drafts folder.  Perhaps one of these bugs should
be marked as a duplicate of the other.

Comment 8

[Scott Couston]

The issue here is that firstly Thunderbird has NO option for encrypting to oneself (in other words a draft message should be encrypted to self - NOT recipient which is what is happening here. What is really needed is an option to either encrypt to self when saving draft messages. In this was a message which has not been sent is encrypted by the Private key part of the encryption certificate 
(what is happening currently is that Thunderbird is using the Public Key which the recipient has.)

The issue with encryption is
1. IF saved as draft == encrypt to self - Private Key - so only you can read it.
2. When message sent ==encrypt to recipient - Public Key - so others can read it.

I think Thunderbird is not using the correct part to encrypt to self in saving a draft and if encrypted to recipient when saved as draft the sender should be able to read it but god knows what happens when send.

Need clarification for

Encrypt to Self - Draft

Encrypt to recipient - Send Message.

This will be an absolute nightmare if the message is both Digitally Signed and Encrypted with a real .X509 certificate or PKI where a CA controls view ability and everything else.

Please get back to me on this - Thanks Scott

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Kai, 
I cannot reproduce this using an x86_32 SeaMonkey trunk build dated 20070123.
Can you still reproduce it?  Is it only a problem on x86_64? 

Scott, 
ever since email encryption was added to mozilla app suite many years ago, 
the core code has always required that, as a condition of being able to 
flag an email for encryption in the composer window, the local user MUST 
have an email encryption cert.  Whenever the email is encrypted, the local
user's cert is always an implicit recipient of the message, even when it 
is just a draft.  The user has no choice about that.  The user cannot choose
to encrypt a message (or draft) only to someone else and not also to himself.
This is done precisely to ensure that the user can read his own drafts and 
the messages in his own "sent" messages folder.  People have complained about
it many times over the years, wishing that they could encrypt an email to 
someone else without having their own cert (private key).  But this policy,
requiring the local user to have his own encryption cert, has been present
since the beginning and no decision has been made to relax that requirement.
It's /conceivable/ to me that this got broken some how, but I rather doubt it.

In any case, I see no reason to believe that Kai's problem is due to any
failure to list his own cert among the recipients.  There's some sort of 
memory/stack corruption going on here, and we need to get to the bottom of it.
If we could reproduce it at will, I am pretty confident that we could fix it
pretty quickly.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

I propose to mark this bug a duplicate of bug 368325.  
Kai, do you concur?

Comment 11

[Scott Couston]

I think a draft composition should be able t be encrypted to self. Consider a user is part the way in composing a restricted message - they save the draft and encrypt to self.... That way if their laptop gets stolen the user's not sent confidential message cannot be read without the unencrypted code. Most email clients now have the ability to encrypt to self - Understand your previous comments.