229242 - mozilla will crash when receiving S/MIME message (Detached Signed Data with CA and client certs)

Status: VERIFIED FIXED

(NSS :: Libraries, defect, P1)

Description

[Louie Zhao]

NISCC (http://www.uniras.gov.uk/vuls/) developed some testcases to detect
vulnerability in S/MIME implementation. Unfortuately, mozilla will crash when
receiving one kind of S/MIME message (Detached Signed Data with CA and client
certs).

Below is the stack when crashing:

#0  0x40799771 in nanosleep () from /lib/i686/libc.so.6
#1  0x40799639 in sleep () from /lib/i686/libc.so.6
#2  0x0806db06 in ah_crap_handler(int) (signum=11) at nsSigHandlers.cpp:149
#3  0x421a885c in nsProfileLock::FatalSignalHandler(int) (signo=11)
    at nsProfileLock.cpp:195
#4  0x400f7a6a in pthread_sighandler () from /lib/i686/libpthread.so.0
#5  <signal handler called>
#6  0x42bb81d2 in NSS_CMSSignedData_GetDigestByAlgTag (sigd=0x80c99e0,
    algtag=SEC_OID_UNKNOWN) at cmssigdata.c:765
#7  0x42bb7daf in NSS_CMSSignedData_VerifySignerInfo (sigd=0x80c99e0, i=0,
    certdb=0x89eb160, certusage=certUsageEmailSigner) at cmssigdata.c:598
#8  0x42b7ff47 in nsCMSMessage::CommonVerifySignature(unsigned char*, unsigned)
    (this=0xbfffe74c, aDigestData=0x0, aDigestDataLen=0) at nsCMS.cpp:371
#9  0x42b7f67f in nsCMSMessage::VerifySignature() (this=0x8bd1438)
    at nsCMS.cpp:174
#10 0x43117545 in MimeCMS_eof (crypto_closure=0x8972fa0, abort_p=0)
    at mimecms.cpp:606
#11 0x43115ee8 in MimeEncrypted_parse_eof (obj=0x85470e0, abort_p=0)
    at mimecryp.cpp:240
#12 0x430e434a in MimeContainer_parse_eof (object=0x8bbfe00, abort_p=0)
    at mimecont.cpp:141
#13 0x430f0e68 in MimeMessage_parse_eof (obj=0x8bbfe00, abort_p=0)
    at mimemsg.cpp:544
#14 0x430fdbad in mime_display_stream_complete (stream=0xfffffffc)
    at mimemoz2.cpp:928
#15 0x4310c410 in nsStreamConverter::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x88cb238, request=0x893dfc0, ctxt=0x0, status=0)
    at nsStreamConverter.cpp:1059
#16 0x42145e4f in nsDocumentOpenInfo::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x875c1e0, request=0x893dfc0, aCtxt=0x0, aStatus=0)
    at nsURILoader.cpp:251
#17 0x41035e78 in nsStreamListenerTee::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x886d9d8, request=0x893dfc0, context=0x0, status=0)
    at nsStreamListenerTee.cpp:65
#18 0x41006150 in nsOnStopRequestEvent0::HandleEvent() (this=0x8b023f8)
    at nsAsyncStreamListener.cpp:319
#19 0x41005965 in nsStreamListenerEvent0::HandlePLEvent(PLEvent*) (
    aEvent=0x8b02408) at nsAsyncStreamListener.cpp:113

Comment 1

[Louie Zhao]

Attachment: patch v1 (for NSS trunk)

Before getting element from "digests", checking it first.

Comment 2

[Louie Zhao]

Notes:

1. The testcases from NISCC is not supposed to be disclosed to the public. so I
didn't provide them here.

2. The stack is generated from mozilla 1.4 workspace since I have no trunk
workspace by hand. I will attach the stack from trunk workspace tomorrow.
Mozilla 1.4 branch and the trunk both have this vulnerability. 

3. Mozilla 1.4 Branch is even worse than trunk because it will crash with many
other testcases from NISCC. I can file bug to discuss the issues for
mozilla1.4_branch if necessary.

Comment 3

[Christopher Blizzard (:blizzard)]

The 1.4 branch needs support, so it would be good to get the patches into it.  I
would just suggest attaching them here, unless the issues are signifigantly
different.

Comment 4

[Wan-Teh Chang]

Thank you for the bug report and the patch.

I believe we have fixed this crash on the NSS trunk,
which is being tested by the Mozilla trunk (1.7a).

Could you get the value of the local variable 'idx'
at cmssigdata.c:765 in stack frame #6?  If 'idx' is
-1, that will confirm that we have already fixed this
crash on the NSS trunk.

For the Mozilla 1.4 branch, I strongly recommend
that you upgrade to the upcoming NSS 3.9 rather
than carrying our NISCC fixes back to the Mozilla
1.4 branch.

Comment 5

[Wan-Teh Chang]

Comment on attachment 137875 [details] [diff] [review]
patch v1 (for NSS trunk)

This patch is for the NSS trunk, not the MOZILLA_1_4_BRANCH.

This patch (for the NSS trunk) is not necessary but is good
defensive programming.	The reason this patch is not
necesssary is that if sigd->digestAlgorithms is not NULL,
then sigd->digests is not NULL unless we ran out of memory.
This point is clear in the following code at cmssigdata.c:1005:

    if (NSS_CMSArray_Add(poolp, (void ***)&(sigd->digestAlgorithms), (void
*)dig
estalg) != SECSuccess ||
	/* even if digest is NULL, add dummy to have same-size array */
	NSS_CMSArray_Add(poolp, (void ***)&(sigd->digests), (void *)digest) !=
S
ECSuccess)
    {
	goto loser;
    }

This is the only place where we create sigd->digestAlgorithms.
It shows that we create sigd->digestAlgorithms and sigd->digests
together.

The reason I went to the trouble of proving that this patch is
not necessary is that the NSS trunk already passed all the NISCC
S/MIME tests without crashes or memory leaks, so it is important
that this patch not invalidate the testing we have done.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Louie,  If you're doing NISCC testing of NSS, be sure to get the sources 
from the tip of the NSS trunk, not earlier than Dec 20, 2003.  Those sources
pass all NISCC tests for SMIME and SSL, where "pass" means no crash and no
memory leak.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Also, Louie, if you experience any more crashes with the trunk code, you
definitely SHOULD disclose exactly which test case you were running when it
failed.  It is sufficient to disclose the directory name and file name of 
the test PKCS7 file for the SMIME tests, or the SSL cert file name for SSL
tests.

Comment 8

[Louie Zhao]

The trunk code will crash when recieving (click to try to display the content)
p7m-sd-dt-c.p7m (Detached Signed Data with CA and client certs) and
p7m-sd-dt-m.p7m (Detached Signed Data with client cert only) and the derived
testcases from the two. Below is the crash stack of trunk (2003-12-29):

#0  0x4079b771 in nanosleep () from /lib/i686/libc.so.6
#1  0x4079b639 in sleep () from /lib/i686/libc.so.6
#2  0x0806fa5a in ah_crap_handler(int) (signum=11) at nsSigHandlers.cpp:135
#3  0x4207c4a0 in nsProfileLock::FatalSignalHandler(int) (signo=11)
    at nsProfileLock.cpp:195
#4  0x400f9a6a in pthread_sighandler () from /lib/i686/libpthread.so.0
#5  <signal handler called>
#6  0x42576b65 in NSS_CMSSignedData_GetDigestValue (sigd=0x8c4cec8,
    digestalgtag=SEC_OID_UNKNOWN) at cmssigdata.c:1036
#7  0x425761b8 in NSS_CMSSignedData_VerifySignerInfo (sigd=0x8c4cec8, i=0,
    certdb=0x8a9d088, certusage=certUsageEmailSigner) at cmssigdata.c:677
#8  0x4253c08d in nsCMSMessage::CommonVerifySignature(unsigned char*, unsigned)
    (this=0xbfffe76c, aDigestData=0x0, aDigestDataLen=0) at nsCMS.cpp:371
#9  0x4253b7c5 in nsCMSMessage::VerifySignature() (this=0x8942c18)
    at nsCMS.cpp:174
#10 0x42febc2d in MimeCMS_eof (crypto_closure=0x8aa5340, abort_p=0)
    at mimecms.cpp:606
#11 0x42fea5d0 in MimeEncrypted_parse_eof (obj=0x89461f8, abort_p=0)
    at mimecryp.cpp:240
#12 0x42fb9baa in MimeContainer_parse_eof (object=0x8b07040, abort_p=0)
    at mimecont.cpp:141
#13 0x42fc6850 in MimeMessage_parse_eof (obj=0x8b07040, abort_p=0)
    at mimemsg.cpp:544
#14 0x42fd3523 in mime_display_stream_complete (stream=0xfffffffc)
    at mimemoz2.cpp:923
#15 0x42fe1042 in nsStreamConverter::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x8b05398, request=0x84b39b8, ctxt=0x0, status=0)
    at nsStreamConverter.cpp:1056
#16 0x420182ac in nsDocumentOpenInfo::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x84b3bf8, request=0x84b39b8, aCtxt=0x0, aStatus=0)
    at nsURILoader.cpp:325


Some values at frame 6

(gdb) p n
$1 = 147123304
(gdb) p sigd
$2 = (struct NSSCMSSignedDataStr *) 0x8c4cec8
(gdb) p sigd->digests
$3 = (struct SECItemStr **) 0x0

Mozilla crash because sigd->digests is NULL. I am not sure what causes this, but
adding a check before that can avoid crash.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Louie,

I'm on vacation here, and don't have access to the NISCC files at the moment,
but IIRC, the file you mention is one of the "base" files, which are supposed
to be error free, the basic test cases from which all the other test cases
are derived.  We've never experienced any failures with those files.

Our copies of all the NISCC files (base files, and other test cases) are pure
binary CMS/PKCS7 files.  They are not base64 encoded, and are not in a MIME
format (e.g. with MIME ContentType, etc.)

I don't know of any way that the mozilla email program could read them as 
they are. To read these files with mozilla's email component you would
presumably have had to construct an actual MIME messsage that contained the
original text, and the detached signature, and then have fed that file into
mozilla (as say a mail folder), or read it from a POP server, or something 
like that.  

My guess is that something about the MIME message you constructed is causing
mozilla's email program to call NSS's libSMIME in a very different way than
it is called by the NSS QA test programs.  I'd like to reproduce exactly
what you've experienced.  So, please zip up the mail file that you created,
(or whatever test file you created), and email me the zip file, along with
instructions on how to reproduce the crash you experienced.  

If you have somehow fed mozilla the raw NISCC PKCS7 file, please advise 
exactly how you did that.

There is a QA test program for NSS's libSMIME, named "cmsutil",
with sources in the misnamed directory nss/cmd/smimetools.  It reads in 
pure binary PKCS7 (or CMS) messages, and parses them.  For detached 
signatures, it takes a command line option that names the file containing
the message text from which the signature is detached.  All the NISCC "SMIME" 
tests we've done were done with cmsutil and the raw NISCC CMS/PKCS7 files,
rather than with Mozilla and MIME message files.  NSS passes all the tests
when done with cmsutil.  But It may be that there remain issues in NSS that
occur when libSMIME is used from mozilla and do not appear when NSS is used
by cmsutil.   If we can have your actual input test case, it should help us
to identify any such differences.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

The function NSS_CMSSignedData_GetDigestValue can, and obviously should 
test sigd->digests before derefencing it here.  It should set some error
code and return NULL in this case.  But this begs the question,
How does the code get to this point without digests?  The cause of that 
problem should also be fixed.  

I wonder if revs 1.22 or 1.24 of cmssigdata.c cause some function that
previously failed (without crashing) when digests was NULL, to now succeed 
in that case, and consequently mozilla takes some path that cmsutil does not.

Also, about the value of "n" in frame 6 (as shown by gdb) as 147,123,304 
- that's utterly bogus.  Valid values should be small integers, e.g. 0..2.  
I doubt that gdb is reporting the proper value for n here.  Even if
sigd->digestAlgorithms was not a properly null terminated array of pointers, 
I doubt the search would go through that many iterations without crashing.

I will look at this bug more on Jan 5, 2004.

Comment 11

[Christopher Blizzard (:blizzard)]

Is it worth it just updating the NSS on the 1.4 branch to something newer to
pick up all these fixes?

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

This bug is waiting for the reporter to add the exact SMIME message that he
created with which he reproduced this bug.  See comment 9 above for more info.

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

Chris, regarding your question in comment 11 above:

Yes, I think it would be advisable to for Moz 1.4 (and all subsequent 
versions) to switch to use NSS 3.9 as soon as it is released (should be
in a few days).  

Comment 14

[Louie Zhao]

Below are detailed enviornment and steps to reproduce this bug:

Enviornment
-----------------------------------------------------------
Build platform target i686-pc-linux-gnu (SuSE Linux)

Build tools
Compiler 	Version 	        Compiler flags
gcc             gcc version 3.2.2       -Wall -W -Wno-unused -Wpointer-arith
-Wcast-align -Wno-long-long -pedantic -pthread -pipe
c++             gcc version 3.2.2 	-fno-rtti -fno-exceptions -Wall -Wconversion
-Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy
-Wno-non-virtual-dtor -Wno-long-long -pedantic -fshort-wchar -pthread -pipe

Configure arguments
--enable-optimize --disable-tests --with-xprint --enable-xinerama --enable-ldap
--enable-crypto --enable-x11-shm --enable-default-toolkit=gtk2 --disable-gtktest
--disable-glibtest --disable-libIDLtest --disable-auto-deps --enable-strip-libs
--enable-xft



Reproduce Steps:
-------------------------------------------------------------
1. Open Perference->Privacy&Security->Certificates->Manage Certificates...
2. In tab "Authorities", import "CA.crt". "TestRootCA" will be added.
3. Edit "TestRootCA", select "This certificate can identify web sites" and "This
certificate can identify mail users"
4. execute "blat.exe empty.txt -to [e-mail] -attach p7m-sd-dt-c.p7m
-server sport-mail1.prc.sun.com -smime -noh2 -subject p7m-sd-dt-c.p7m -penguin
-f testclient@testrootca" on another host(Windows)
5. Open MailNews to get the mail. You can see the mail is received and in the
INBOX list.
6. Try to open this mail, Mozilla always crash.


Notes:
-------------------------------------------------------------------------
1. My copies of NISCC files (base files, and other test cases) are all pure
binary CMS/PKCS7 files.

Comment 15

[Nelson Bolyard (seldom reads bugmail)]

Thanks for the info.  I see now that you're using the specially modified 
version of blat, developed at NISCC to facilitate this testing.  We didn't
use blat in our testing because:
a) supposedly all it did was send the message to a server via SMTP, 
b) Given that there are ~ a million test messages, we didn't want to have to
send millions of messages to a server, and fetch them back.  We're not trying
to test SMTP or POP or IMAP, after all.  We're testing CMS/PKCS7 parsing and 
processing.  We didn't want to overload a mail server every time we do NISCC
SMIME testing, so our testing bypasses the SMTP and IMAP/POP protocols.

But I see now that this modified blat produces messages that (AFAIK) are not
formatted the way that most S/MIME programs format them.  So, the output of
blat is also testing the MIME parser, which is part of mozilla but isn't part of
NSS.  I tried to get this blat to format the messages as multipart/signed, but
the best I could get out of it was multipart/mixed. 

Comment 16

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v2 for NSS trunk

After getting past the crash seen above, another crash happens in 
NSS_CMSContentInfo_GetContent().

This patch fixes both crashes, and also sets the appropriate error code in 
the case of the first crash.

This patch also makes it easier to debug values in the stack.

With this patch in place, I can read all the messages I created with 
the NISCC version of blat, without crashing.  

Reporter, would you be willing to test this patch also?

Comment 17

[Nelson Bolyard (seldom reads bugmail)]

Here are some additional improvements that could be made to this code:

a) NSS_CMSMessage_GetContent could test cinfo for NULL before passing it 
   to NSS_CMSContentInfo_GetInnerContent.

b) NSS_CMSContentInfo_GetContent could set an error code when tag is 
   SEC_OID_UNKNOWN

Comment 18

[Louie Zhao]

This patch works ok for me (using the latest trunk).

Comment 19

[Wan-Teh Chang]

Comment on attachment 138466 [details] [diff] [review]
patch v2 for NSS trunk

r=wtc.	Bob, could you review this patch, too?

Nelson, I have a suggested change and two questions.

1. Please confirm that your changes to
NSS_CMSContentInfo_GetInnerContent and
NSS_CMSMessage_GetContent are "coding for
debuggability" changes that do not change
the behavior of these functions.

2. In your change to cmssigdata.c, you
should parenthesize the test
sigd->digestAlgorithms == NULL as well.

3. Did you see an utterly bogus value of "n"
in NSS_CMSSignedData_GetDigestValue such as
the one shown in comment 8?  If so, do you
know how "n" could assume that value?

Comment 20

[Robert Relyea]

Comment on attachment 138466 [details] [diff] [review]
patch v2 for NSS trunk

patch looks good.

I only have one possible issue... Should the cinfo->contentTypeTag->offset
change to a call to NSS_CMSContentInfo_GetContentTypeTag(cinfo)?
The change will make the code more readable, but also changes the behavior in
the following ways:

1) Currently if NSS_CMSContentInfo_GetContent () is called before any call to
NSS_CMSContentInfo_GetContentType() is called, then GetContent will crash
(pre-Nelson's patch) or fail (post-Nelson's patch). [good]

2) In the case where we have an invalid content type, we will make extra calls
to SECOID_FindOID(). [bad]

Comment 21

[Nelson Bolyard (seldom reads bugmail)]

In answer to Wan-Teh's 2 questions (Q1 and Q3) in comment 19 above,

A1. Yes, I confirm, the only changes to these functions is to make them 
more easily debugged, by storing the results returned by function calls 
in automatic variables before using them in subsequent calls/switches.

A3. In my testing with mozilla on Win2k, I got n==0.  I conclude that the
value reported by gdb was wrong.  The patch to this function does prevent
this crash.

BTW, The crash that Louie experienced in NSS_CMSSignedData_GetDigestValue 
occurs when the root CA is trusted for email.  The crash that I experienced 
in NSS_CMSContentInfo_GetContent occurs when the root CA is NOT trusted for
email.  I wonder if there are other paths with problems, and what conditions
might trigger them.

Comment 22

[Nelson Bolyard (seldom reads bugmail)]

/cvsroot/mozilla/security/nss/lib/smime/cmscinfo.c,v  <--  cmscinfo.c
new revision: 1.6; previous revision: 1.5

/cvsroot/mozilla/security/nss/lib/smime/cmsmessage.c,v  <--  cmsmessage.c
new revision: 1.5; previous revision: 1.4

/cvsroot/mozilla/security/nss/lib/smime/cmssigdata.c,v  <--  cmssigdata.c
new revision: 1.27; previous revision: 1.26

Marking fixed.

Comment 23

[Louie Zhao]

Can someone help to port the fixing for S/MIME security hole in NSS 3.9 to
mozilla 1.4 branch ? Or can someone give a list of the bug number for the fixing
of S/MIME security hole? 

Thanks very much.

Comment 24

[Nelson Bolyard (seldom reads bugmail)]

Louie:

IMO, the solution is to use NSS 3.9 with moz 1.4.  

I would not recommend that anyone try to back port all the NSS 3.9 changes
(there are MANY) to moz 1.4 branch, because if someone attempts that, and
that branch doesn't work properly, that branch will be unsupported.  
the NSS team supports the NSS numbered releases (e.g. 3.9) and nothing else.

It should work to either 
a) take the NSS 3.9 binaries from ftp.mozilla.org and place those shared libs
into a moz 1.4 directory, or 
b) change moz 1.4 to pull NSS (mozilla/security/nss and
mozilla/security/coreconf) from the NSS 3.9 tag. (e.g. NSS_3_9_RTM) and build
moz 1.4 with those sources.

Comment 25

[Wan-Teh Chang]

The NSS team recommends that the Mozilla 1.4 branch use
NSS_3_9_RTM rather than porting specific fixes to the
Mozilla 1.4 branch.

You need to first get drivers' approval.  I believe the
drivers are already considering this proposal, but it
may be a good idea to formally open a bug.

Then, you can just slam dunk the contents of NSS_3_9_RTM
onto the MOZILLA_1_4_BRANCH.  Note that some files need
to be cvs added and some cvs removed.

Comment 26

[Christopher Blizzard (:blizzard)]

Would you recommend that for the Mozilla 1.0 branch as well?

Comment 27

[Wan-Teh Chang]

Chris: no, we do not recommend NSS 3.9 for the Mozilla 1.0
branch. The Mozilla 1.0 branch is using NSS 3.5, which is
too different from NSS 3.9.  You have the following obstacles:
Mac OS Classic support, cert8.db upgrade, and the softoken
.chk files for FIPS mode.

The Mozilla 1.4 branch is using NSS 3.8.x, which is very
close to NSS 3.9.  None of the above three obstacles is
an issue for the upgrade from NSS 3.8.x to 3.9.  This is
one reason we recommend NSS 3.9 for the Mozilla 1.4 branch.

Comment 28

[Daniel Veditz [:dveditz]]

I believe mkaply has or will soon land NSS 3.9 in time for 1.4.2, setting the
blocking flag as a reminder to add the "fixed1.4.2" keyword when this has in
fact happened. Please clear the blocking flag if we're not doing that.

Comment 29

[Daniel Veditz [:dveditz]]

Looks like mkaply landed NSS 3.9 on the 1.4 branch on Jan 23

Comment 30

[Tracy Walker [:tracy]]

verified fixed per L.Z.'s comment on the trunk  other than a small change to the
path there, the remainder of the bug discussed 1.4 branch implementation on NSS 3.9

Please reopen if this is not working on the trunk or 1.7 

------- Additional Comment #18 From Louie Zhao  2004-01-05 23:43 PDT  [reply]
-------

This patch works ok for me (using the latest trunk).

Comment 31

[Daniel Veditz [:dveditz]]

Adding Jon Granrose to CC list to help round up QA resources for verification

Comment 32

[Daniel Veditz [:dveditz]]

Is there any reason the confidential flag can't be cleared now?

Comment 33

[Wan-Teh Chang]

Status of all supported Mozilla clients today:

Mozilla 1.4.x and 1.7.x series:

    Mozilla 1.4 and 1.4.1 are vulnerable.
    Mozilla 1.4.2 - 1.4.4 are not vulnerable.
    Mozilla 1.7 - 1.7.7 are not vulnerable.

Firefox 1.0 - 1.0.3 are not vulnerable.

Thunderbird 1.0 - 1.0.2 are not vulnerable.

Comment 34

[Nelson Bolyard (seldom reads bugmail)]

Wan-Teh, Thanks for this info.  Are any of 1.5.x and 1.6.x vulnerable?

Comment 35

[Wan-Teh Chang]

Mozilla 1.5 and 1.6 are not the so-called long-lived branches.
So I didn't look into them.

Mozilla 1.5 - 1.5.1 are vulnerable.

Mozilla 1.6 is vulnerable.