1507174 - crash in NSS_CMSContentInfo_GetContentTypeTag

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Hanno Boeck]

Attachment: nss-crasher

Certain inputs to the CMS function can crash nss in NSS_CMSContentInfo_GetContentTypeTag(). Similarly to #1507135 this can be used to create thunderbird mails that crash the mail client or firefox addons that crash on installation.

This can also easily be reproduced with cmsutil on the commandline:

cmsutil -D -i [input] -d /tmp/

I'll attach a sample input.

With an ASAN build of nss I get this stack trace:
==24556==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000068 (pc 0x7f758e11e117 bp 0x00000000000d sp 0x7ffd65f411d0 T0)
==24556==The signal is caused by a READ memory access.
==24556==Hint: address points to the zero page.
    #0 0x7f758e11e116 in NSS_CMSContentInfo_GetContentTypeTag /mnt/ram/n2/nss-3.40/nss/lib/smime/cmscinfo.c:285:16
    #1 0x7f758e11e116 in NSS_CMSContentInfo_Destroy /mnt/ram/n2/nss-3.40/nss/lib/smime/cmscinfo.c:54
    #2 0x7f758e11e25a in NSS_CMSContentInfo_Destroy /mnt/ram/n2/nss-3.40/nss/lib/smime/cmscinfo.c:66:13
    #3 0x7f758e1362d2 in NSS_CMSSignedData_Destroy /mnt/ram/n2/nss-3.40/nss/lib/smime/cmssigdata.c:88:5
    #4 0x7f758e11e20e in NSS_CMSContentInfo_Destroy /mnt/ram/n2/nss-3.40/nss/lib/smime/cmscinfo.c:60:13
    #5 0x7f758e130e4d in NSS_CMSMessage_Destroy /mnt/ram/n2/nss-3.40/nss/lib/smime/cmsmessage.c:99:5
    #6 0x7f758e1275f9 in NSS_CMSDecoder_Finish /mnt/ram/n2/nss-3.40/nss/lib/smime/cmsdecode.c:713:9
    #7 0x5018e3 in decode /mnt/ram/n2/nss-3.40/nss/cmd/smimetools/cmsutil.c:196:12
    #8 0x4fc921 in main /mnt/ram/n2/nss-3.40/nss/cmd/smimetools/cmsutil.c:1454:24
    #9 0x7f758d98b4ca in __libc_start_main (/lib64/libc.so.6+0x234ca)
    #10 0x41eee9 in _start (/mnt/ram/n2/nss-3.40/nss/cmd/smimetools/Linux4.19_x86_64_clang-7_glibc_PTH_64_DBG.OBJ/cmsutil+0x41eee9)

For some reason the cinfo pointer ends up being 0x68.

This was co-found by Damian Poddebniak.

(I believe this has been previously reported here https://bugzilla.mozilla.org/show_bug.cgi?id=757064 as a Thunderbird bug but closed because it couldn't be analyzed properly)

Comment 1

[Franziskus Kiefer [:franziskus]]

Thanks for reporting Hanno. There seem to be a couple places that don't check `cinfo` properly here before working on it.

Comment 2

[Daniel Veditz [:dveditz]]

The specific crash looks like a null deref, but if it looks more serious when we find the cause (not checking a cinfo might mean it could do worse things with the right values?) we can up the severity.

Comment 3

[J.C. Jones [:jcj] (he/him)]

I have the patch for this in-progress, the main hang-up being getting some smime gtests working correctly.

Comment 4

[Hanno Boeck]

@jjones: Can you share the patch?

I hereby announce that unless it's fixed earlier I plan to make this public by the end of January.

Comment 5

[J.C. Jones [:jcj] (he/him)]

(In reply to Hanno Boeck from comment #4)
> @jjones: Can you share the patch?
> 
> I hereby announce that unless it's fixed earlier I plan to make this public
> by the end of January.

I understand. I'll be back from PTO on Monday and will get the patch posted ASAP shortly thereafter.

Comment 6

[J.C. Jones [:jcj] (he/him)]

Attachment: Bug 1507174 - Add more null checks to the smime CMS functions r=mt

This was a starting-attempt at protecting against a null deref caused by the reporter's
invalid CMS input, and while the NULL checks should have been there in the first place,
the actual issue is the ASN1 streaming decoder, so the patch doesn't actually fix the
problem.

I had been under the impression we'd trimmed out all code that could reach the streaming
decoder from Firefox, but the CMS functions here are indeed in
mozilla-central/security/nss.symbols...

I've spent a bit of time trying to make sense of how to catch the decoder's
mistake but I haven't quite figured out how, yet. Since I promised an update
though, I'm going ahead and posting this for feedback, if anyone has some to
share.

Obviously, the test can't land until we fix the problem. Note that you'll need
an ASAN build to catch it reliably.

Comment 7

[J.C. Jones [:jcj] (he/him)]

Note: my update to the patch still doesn't resolve the issue.

The general problem here goes as follows: The attached nss-crasher declares a CMS structure of a nice long length. The CMS decoder uses the NSS streaming asn1 decoder to read the structure, and it dutifully constructs a cmsg object.

During the NSS_CMSDecoder_Finish, NSS_CMSMessage_Destroy is called on that cmsg object which ultimately results in going off the rails because it's decoding invalid data.

I've unfortunately only had a limited number of hours to dig through this -- being my first real bug in NSS in a long while, I had to wrestle with ASAN and getting lldb to work first. Because of that I don't actually have a handle on where the streaming decoder populates the cmsg structure and where I could sanity-check the length. I'm going to swap to bug 1507135 for the moment as Hanno has provided a patch for that one.

Comment 8

[J.C. Jones [:jcj] (he/him)]

[deleted]

Comment 9

[Hanno Boeck]

Attachment: crash2.eml

Example mail triggering the crash in Thunderbird.

Comment 10

[J.C. Jones [:jcj] (he/him)]

I'm planning to ship the fix for this in NSS 3.42.

Comment 11

[J.C. Jones [:jcj] (he/him)]

Note for Relman: Requesting ESR tracking for Thunderbird. See Bug 1520826 and bug 1507135.

Comment 12

[Julien Cristau [:jcristau]]

Tracking for 60.5esr.

Comment 13

[J.C. Jones [:jcj] (he/him)]

https://hg.mozilla.org/projects/nss/rev/08d1b0c1117f
NSS_3_36_BRANCH: https://hg.mozilla.org/projects/nss/rev/54b65b98450f

Comment 14

[Julien Cristau [:jcristau]]

Fixed in esr60 as part of 1520826.

Comment 15

[J.C. Jones [:jcj] (he/him)]

Embargo over; tests landed in bug 1521174.

Landed as https://hg.mozilla.org/projects/nss/rev/08d1b0c1117f in trunk.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

No advisory for Firefox here. Kai, maybe Thunderbird will want an advisory written by you when it releases?

Comment 17

[Kai Engert (:KaiE:)]

Hi Al, given that this bug and bug 1507135 are very similar in scope, I had assumed that a single advisory for both is sufficient.

In the tracking bug for the advisory for CVE-2018-18513 (bug 1533300) I mentioned both bugs.

Comment 18

[Al Billings [:abillings - ex-MoCo]]

(In reply to Kai Engert (:kaie:) from comment #17)

Hi Al, given that this bug and bug 1507135 are very similar in scope, I had assumed that a single advisory for both is sufficient.

In the tracking bug for the advisory for CVE-2018-18513 (bug 1533300) I mentioned both bugs.

Ah, right. Ok. Sounds good.