1507135 - (CVE-2018-18508) null pointer / crash in NSS_CMSMessage_IsSigned (can be used to crash Firefox addon installer or Thunderbird SMIME verification)

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Hanno Boeck]

Attachment: proposed patch

We discovered a null pointer deref in NSS_CMSMessage_IsSigned().

This can be triggered by using a CMS signature that contains a signedData OID and nothing else.

This can be used to crash the Firefox addon installer by crafting such a signature, I'll attach an example XPI file demonstrating that. It can also be used to crash thunderbird with an SMIME signature.

I'll attach a stack trace from a firefox asan build. (You may have gotten several such crashes via the nightly/asan build that got sent automatically, I wasn't aware that this happens immediately.)

I'll also attach a patch that I believe fixes this, it adds a check to that function verifying that cinfo->content.signedData is valid.

I discovered this together with Damian Poddebniak (in case you want to credit us).

Comment 1

[Hanno Boeck]

Attachment: asan crash / stack trace

Comment 2

[Hanno Boeck]

Attachment: crash.xpi / firefox extension PoC

Comment 3

[Franziskus Kiefer [:franziskus]]

Thanks for reporting Hanno. I can reproduce the issue and the proposed patch should fix it.
Dipen can you add a test and make a patch?

Comment 4

[Hanno Boeck]

Hi, can I get an update when you plan to fix this?

Comment 5

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

With luck Dipen can get to this in the 66 cycle. If he can't here in December, I'll take it on in January. (It's just the test that really needs doing.)

Thanks for the ping on this Hanno.

Comment 6

[Hanno Boeck]

I'm a bit surprised by the lack of activity here, given in the meantime there was a new nss release without a fix and I provided a trivial patch to fix this.

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

Comment 7

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

Since Dipen's now unavailable, as I mentioned in comment #5 I'm taking on writing the test.

Comment 8

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

Attachment: Bug 1507135 - Add additional null checks to CMS message functions r=mt

This changeset adds some tests for S/MIME CMS signatures, and adds
null checks per Hanno's disclosure.

This one is straightforward. The additional null checks on the cmsmessage.c
methods all return safe values in at least mozilla-central, but I didn't
confirm against Thunderbird (yet).

Note: this changeset will conflict with bug 1507174, but the merge should be
clean.

Comment 9

[Daniel Veditz [:dveditz]]

The "crash.xpi" testcase doesn't crash me in Firefox 64, 65, or 66 (on Mac, if it matters).

Do we have a PoC of the ability to crash Tbird in a non-ASAN build? That's the more serious DoS anyway.

Comment 10

[Daniel Veditz [:dveditz]]

Didn't crash from the attachment link, but from a local saved file:/// url it definitely did:
bp-e033420c-aea3-4d6c-bd56-d4ea80190114

Don't know why those would be different but that's for another day.

Comment 11

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

Wayne, are you going to want an ESR uplift of this patch (and presumably bug 1507174)? If so, we should mark this for ESR tracking.

Comment 12

[Hanno Boeck]

Attachment: crash1.eml

Sample email, it's just taking a basic smime mail and copying the base64-encoded sig into the signature part.

sendmail [victim] < [mail].eml
should do.

Comment 13

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

I'm planning to ship the fix for this (and bug 1507174) in NSS 3.42 and at this point will not plan to uplift to ESR (comment 11).

Hanno, how would you and Damian like to be credited in the release notes?

Comment 14

[Wayne Mery (:wsmwk)]

(In reply to J.C. Jones [:jcj] (he/him) from comment #11)

Wayne, are you going to want an ESR uplift of this patch (and presumably bug 1507174)? If so, we should mark this for ESR tracking.
Thanks for asking. From stability POV no - given that this crash signature isn't seen in the wild. From a vuln/security POV I defer to kaie.

Comment 15

[Hanno Boeck]

(In reply to J.C. Jones [:jcj] (he/him) from comment #13)

Hanno, how would you and Damian like to be credited in the release notes?

Just mention our full names (Hanno Böck and Damian Poddebniak).

Comment 16

[Kai Engert (:KaiE:)]

J.C., you say you target NSS 3.24, so Firefox 66, with 66 beta around Jan 29, and 66 release around mid March, right?

Hanno, are you still planning to go public end of January? It would mean that Firefox stable wouldn't be fixed until mid March. Is this an issue for Firefox users, with the issue being public? Maybe not, because most users get only reviewed XPIs from mozilla sites?

For TB the issue seems to be worse, usable for denial-of-service attacks on thunderbird users. If thunderbird potentially remembers a bad email as currently being selected, and starts with it, it might crash immediately again, with no way out? If true, this could even be a permanent DOS until fixed.

J.C., I'd prefer an uplift of the fix to ESR. Taking both issues should be fine, as the code is close to each other, and mostly is about null checks, so it should be fine. Would you like help with the uplifts?

If Hanno goes public by end of January, we'd need a new NSS 3.36.x release for 60.5 and Jan 29 release, right?

If you're worried about Firefox, we'd also require an NSS 3.41.x for Firefox 65, unless Hanno delays going public to mid march.

Comment 17

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

We're not worried about Firefox for either this or bug 1507174. I think there's reason to be worried about Thunderbird though, which is why I wanted to know if we need an ESR uplift. The time is late on that, so let's mark now.

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

Comment 18

[Hanno Boeck]

How long would we have to delay disclosure to give Thunderbird a chance to have a fix?

Comment 19

[Julien Cristau [:jcristau]]

Tracking for 60.5esr.

Comment 20

[Wayne Mery (:wsmwk)]

(In reply to Hanno Boeck from comment #18)

How long would we have to delay disclosure to give Thunderbird a chance to have a fix?

I would be comfortable with March 1 - which gives time for our release testing and time in production to ensure there are no regressions which cause us to back out the fix.

Comment 21

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

(In reply to Wayne Mery (:wsmwk) from comment #20)

(In reply to Hanno Boeck from comment #18)

How long would we have to delay disclosure to give Thunderbird a chance to have a fix?

I would be comfortable with March 1 - which gives time for our release testing and time in production to ensure there are no regressions which cause us to back out the fix.

Wayne,

Does that mean you would like this to wait beyond this next ESR release? As Martin pointed out, landing the tests at least discloses the vulnerability.

We could land the patches w/o the tests, but it's still a risk.

Comment 22

[Kai Engert (:KaiE:)]

I've had a chat with Wayne to explain the situation, especially the complexity around NSS release timing.

Usually, Thunderbird needs more time to rebase to a newer Firefox, because things can go wrong. For the Firefox ESR 60.5 release that is planned for January 29, it could take around 2 weeks before a TB 60.5 release can be safely ready, or longer, which is why Wayne had suggested March 1st.

I'd like to suggest an alternative plan, which will require the Thunderbird drivers to agree (and I'll ping them on tb-drivers shortly). But I'd like to explain the suggestion here:

• we release NSS 3.42 and NSS 3.36.7 soon

• FF ESR 60.5 picks up NSS 3.36.7

• we release an additional Thunderbird 60.4.1 release,
  scheduled for Jan 29, the same day as FF ESR 60.5

• TB 60.4.1 picks up NSS 3.36.7

• Thunderbird team releases TB 60.5 later, when the rebase is ready and tested

Comment 23

[Jorg K (CEST = GMT+2)]

No sorry, that won't work. I've been moving everything forward so we can release TB 60.5 based on mozilla-esr at 60.5. Releasing anything with 60.4.x is extremely inconvenient since it would involve branches and such. I won't mention that mozilla-esr have reformatted 10.000+ C++ files so any backport to something based on 60.4.1 might involve a difficult rebase.

What's the issue here? We can build TB 60.5 on the day mozilla-esr 60.5 is ready, and knowing Mozilla, that will be some days before the FF 60.5 release date. Ask RyanVM about those details.

Comment 24

[Jorg K (CEST = GMT+2)]

BTW, TB 60.5 is ready now. One final merge of m-esr60 at 60.5 into our branch and done.

Comment 25

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

(In reply to Jorg K (GMT+1) from comment #23)

I won't mention that mozilla-esr have reformatted 10.000+ C++ files so any backport to something based on 60.4.1 might involve a difficult rebase.

This, at least, is not a problem as NSS was already clang-formatted since 2017.

I think the only issue is when to cut an NSS release. If we want this to be in ESR 60.5, then I land these patches and cut the release Saturday, then the release folks to call the upgrade script on NSS against the ESR branch.

If we want slightly more safety, I can land the patches without the tests, and add the tests in later.

Comment 26

[Jorg K (CEST = GMT+2)]

Ready when you are ;-)

Comment 27

[Wayne Mery (:wsmwk)]

(In reply to Jorg K (GMT+1) from comment #24)

BTW, TB 60.5 is ready now. One final merge of m-esr60 at 60.5 into our branch and done.

You personally may be ready from a code POV, but testing and shipping doesn't happen in a day. And in Bug 393302 it seems you are poised to include the patch which I have already stated I am not in favor of shipping without extended beta testing. So as it looks today I am not committing to shipping 60.5.0 quickly.

Comment 28

[Jorg K (CEST = GMT+2)]

As I said, once m-esr60 at 60.5 is ready, we can build and test, most likely before the release date. Surely FF build and test before. The MAPI bug isn't included so far, so let's keep this out of the discussion. If you wish, I can provide ready-to-go code on the 21st, then you can build and test during 8 days.

Comment 29

[Hanno Boeck]

Delaying disclosure until March 1st is okay with us.

Comment 30

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

https://hg.mozilla.org/projects/nss/rev/5e70b72131ac
NSS_3_36_BRANCH https://hg.mozilla.org/projects/nss/rev/cf8755bd82c6

Comment 31

[Julien Cristau [:jcristau]]

Fixed in esr60 as part of 1520826.

Comment 32

[Wayne Mery (:wsmwk)]

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

...
For TB the issue seems to be worse, usable for denial-of-service attacks on thunderbird users. If thunderbird potentially remembers a bad email as currently being selected, and starts with it, it might crash immediately again, with no way out? If true, this could even be a permanent DOS until fixed.

I can confirm this causes DOS. After I mailed the testcase to myself, I constantly crashed on startup. Crashes stopped only after I removed the message from Thunderbird by manually editting the folder containing the message.

So I think for THunderbird purposes this bug should be categorized as sec-moderate or sec-high.

Comment 33

[Kai Engert (:KaiE:)]

Let's clarify the plan for Thunderbird BETA.
Current TB 65 beta is vulnerable.
As soon as TB beta is upgraded to 66, it will be fine.

How soon can TB beta branch be uplifted to 66, and TB 66 beta shipped?

Comment 34

[Kai Engert (:KaiE:)]

When do we intend to publish the security advisory for TB 60.5 that mentions the issue?
Would this usually be done immediately on Jan 29?

If yes, should this be coordinated with the date on which Hanno goes public?

If Mozilla publishes a security advisory on Jan 29, then TB Beta users will be affected until upgraded to TB Beta 66.

Comment 35

[Jorg K (CEST = GMT+2)]

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

How soon can TB beta branch be uplifted to 66, and TB 66 beta shipped?
TB 66 is currently busted. I don't think it makes sense to ship a TB 66 beta until the functional issues are addressed.

I suggest to build a TB 65 beta 4 with a branch on mozilla-beta and this branch can include the new NSS library.

Comment 36

[Jorg K (CEST = GMT+2)]

Where/when/which changeset/which bug was NSS 3.36.7 landed on mozilla-central (so I can put this onto the mozilla-beta branch)?

Comment 37

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

(In reply to Jorg K (GMT+1) from comment #36)

Where/when/which changeset/which bug was NSS 3.36.7 landed on mozilla-central (so I can put this onto the mozilla-beta branch)?

It landed into ESR: https://hg.mozilla.org/releases/mozilla-esr60/rev/c13c0781ad99

See Bug 1520826

Comment 38

[Julien Cristau [:jcristau]]

3.36.7 landed on esr60 not central. AIUI central (66) will get the fix from 3.42 when that is released. 65 (using nss 3.41) will not.

Comment 39

[Jorg K (CEST = GMT+2)]

Sure. But you're saying that mozilla66 is not vulnerable, but mozilla65 is. So what needs to go back onto mozilla65 from mozilla66? I've created a mozilla-beta branch and would like to do that backport/uplift. Yet, I can't see it:
https://hg.mozilla.org/mozilla-central/log/tip/security/nss/lib/smime/cmsmessage.c.

For example cmsmessage.c wasn't changed in ages on mozilla-central.

Comment 40

[Jorg K (CEST = GMT+2)]

(In reply to Julien Cristau [:jcristau] from comment #38)

3.36.7 landed on esr60 not central. AIUI central (66) will get the fix from 3.42 when that is released. 65 (using nss 3.41) will not.

OK, so where is the fix in 66?

Comment 41

[Kai Engert (:KaiE:)]

Jörg, each version of Mozilla uses its own corresponding NSS version. Mozilla and NSS are released with an aligned schedule. See http://kuix.de/mozilla/versions/ for an up to date list of versions.

In other words:

• Moz 60.x uses NSS 3.36.x
• Moz 65.x uses NSS 3.41.x
• Moz 66.x uses NSS 3.42.x

The NSS snapshot used for Moz 60.x cannot be used for 65.x, because it's too old.

Also, we usually avoid to cherry-pick individual NSS changes into release/beta branches, because that can cause problems for Linux distributions.

If we want to fix TB 65 beta (because TB 66-beta takes too long), then we should uplift the NSS fix into the NSS 3.41.x branch, make a new NSS 3.41.x release, and uplift that into the Mozilla 65.x beta branch.

Comment 42

[Kai Engert (:KaiE:)]

(In reply to Jorg K (GMT+1) from comment #39)

Sure. But you're saying that mozilla66 is not vulnerable, but mozilla65 is.

No, that's a misunderstanding.

So what needs to go back onto mozilla65 from mozilla66?

It's need a newer NSS snapshot from the NSS 3.41.x branch. We haven't yet uplifted the fix to that branch. That's a todo, if we want to fix the TB 65 branch.

Comment 43

[Jorg K (CEST = GMT+2)]

OK.

Comment 44

[Kai Engert (:KaiE:)]

I've discussed with J.C., we agreed to land the fixes from both bugs on the NSS 3.41.x branch and make a new release. I've agreed to help with that, and do that today.

I don't know if we can get approval to update mozilla-beta. If not, we can use a forked branch of mozilla-beta for Thunderbird. Jörg, you said you already created a branch, what's the name of that branch? Do you want me to uplift the minor NSS change into that branch?

Comment 45

[Kai Engert (:KaiE:)]

uplift the NSS 3.41.x release (which will be a minor change, just those fixes)

Comment 46

[Jorg K (CEST = GMT+2)]

I've created a branch on my local machine, it's empty so far.

It's branched off
9876a3bd6201 Jonathan Kew — Bug 1517937 - Move JhengHei to the end of zh-TW sans-serif font prefs on late-beta/release so that previous default of PMingLiu will take precedence for now. r=m_kato, a=RyanVM FENNEC_65_0b13_BUILD1 FENNEC_65_0b13_RELEASE FIREFOX_RELEASE_65_BASE

and needs to be called THUNDERBIRD650b4_2019012201_RELBRANCH - so long version: 65.0b4 2019 01 22 01, 01 being some order number.

Ignore the following if you know it already:

on m-b:

hg pull -r 9876a3bd6201 (saves pulling all the mozilla66 stuff which was uplifted already)
hg up -r 9876a3bd6201
do your stuff, commit with DONTBUILD
hg push --new-branch -b THUNDERBIRD650b4_2019012201_RELBRANCH ssh://[e-mail]/releases/mozilla-beta/

Comment 47

[Wayne Mery (:wsmwk)]

This seems like a lot of work when we could just ship 66 with a fix. Is 66 so bad that we expect it won't build?

Comment 48

[Jorg K (CEST = GMT+2)]

(In reply to Wayne Mery (:wsmwk) from comment #47)

This seems like a lot of work when we could just ship 66 with a fix.

Sadly so. We've been working on the assumption of "ship when ready", but security bugs/fixes/disclosures don't play that game. Uplifting some NSS fixes doesn't appear to be an excessive amount of work.

Is 66 so bad that we expect it won't build?

Well, I've switched off 40 tests over in bug 1518823 and six more in bug 1521016. While we're hoping to fix the latter in time, no one has even looked at what is broken in the former.

So shipping a working and not vulnerable TB 65 beta 4 seems to be the safest choice.

Comment 49

[Wayne Mery (:wsmwk)]

So shipping a working and not vulnerable TB 65 beta 4 seems to be the safest choice.
That sounds fine to me

Comment 50

[Kai Engert (:KaiE:)]

Good, now we all agree. Let's fix TB beta now. I'll make the NSS 3.41.x release and will follow Jörg's uplift instructions from comment 46.

Comment 51

[Jorg K (CEST = GMT+2)]

Damn, I forgot :-(

On mozilla-beta:

hg pull -r 9876a3bd6201 (saves pulling all the mozilla66 stuff which was uplifted already)
hg up -r 9876a3bd6201

hg branch THUNDERBIRD650b4_2019012201_RELBRANCH  <=== this will start a branch at where you are.

do your stuff, commit with DONTBUILD

hg out  -  check that your changeset is on the branch

hg push --new-branch -b THUNDERBIRD650b4_2019012201_RELBRANCH ssh://[e-mail]@hg.mozilla.org/releases/mozilla-beta/

I can do it if you prefer, but I'd need a patch.

Comment 52

[Kai Engert (:KaiE:)]

Jörg, ok, I'll let you upgrade the Thunderbird branch. But we don't upgrade Mozilla branches by applying NSS patches. We upgrade Mozilla branches by running a python script, which pulls in an updated NSS release snapshot. We've been doing that for years. Once I have the 3.41.x ready, I'll give you the commands you need to run locally. You can then run "hg diff", and you'll see the code changes, plus a few NSS version numbers changes. Please wait until I send you the instructions.

Comment 53

[Jorg K (CEST = GMT+2)]

I've tried TB 60.5.0 we've just built. Doesn't crash with attachment 9036650 [details] :-)

Comment 54

[Kai Engert (:KaiE:)]

Attachment: uplift-3.41.1-to-tb-65-beta.patch

Jörg, I've decided to give you a full patch. This one applies on top of revision 9876a3bd6201 on mozilla-beta as you explained in comment 51.

(It updates NSS to the NSS_3_41_1_RTM release tag, and it also changes old-configure.in to require the new 3.41.1 release when building with NSS system libraries on Linux, this will remind anyone building this for a Linux distribution with separate NSS that they need to get that NSS.)

I suggest the following commit command:

hg commit -u "Kai Engert <[e-mail]>" -m "Bug 1507135, update Thunderbird 65 Beta to NSS 3.41.1, a=jorkg, UPGRADE_NSS_RELEASE, DONTBUILD"

Comment 55

[Jorg K (CEST = GMT+2)]

Perfect, I'll do it later today and start a beta build. I'll announce on tb-drivers when ready. Enough time to get this shipped on 29th Jan.

Comment 56

[Jorg K (CEST = GMT+2)]

https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=ef5478ee0f0a315fcbed7466cb5a671dbef7b9ce

ef5478ee0f0a Kai Engert — Bug 1507135 - update Thunderbird 65 beta 4 release branch to NSS 3.41.1. a=jorgk UPGRADE_NSS_RELEASE, DONTBUILD on THUNDERBIRD650b4_2019012301_RELBRANCH

Comment 57

[Hanno Boeck]

I'm a bit surprised this is not mentioned in the Thunderbird security advisory given its impact of making TB permanently unusable.

Comment 58

[Kai Engert (:KaiE:)]

(In reply to Hanno Boeck from comment #57)

I'm a bit surprised this is not mentioned in the Thunderbird security advisory given its impact of making TB permanently unusable.

IIUC the security advisory for this bug is delayed until the agreed embargo date, March 1st.

Comment 59

[Hanno Boeck]

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

IIUC the security advisory for this bug is delayed until the agreed embargo date, March 1st.

Okay, given that this was already in the NSS release notes I thought any embargo is obsolete. (Not a fan of half-embargos, but okay, let's go with it.)

Comment 60

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

Al, can we get a CVE for this (and lump in Bug 1507174 I think)?

Comment 61

[Julien Cristau [:jcristau]]

JC, looking at the nss repo it looks like this is on nss trunk, 3.41.1 and 3.36.7, but not the 3.42 branch, even though it's listed at https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.42_release_notes#Bugs_fixed_in_NSS_3.42

Am I missing something?

Comment 62

[Kai Engert (:KaiE:)]

Looks like Julien is correct, 3.42 was branched just before the trunk commit, and it hasn't been landed on 3.42.
https://hg.mozilla.org/projects/nss/graph

Comment 63

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

Yes, agreed. hg log -G's ascii art failed me. I'll add more notes onto the NSS release management wiki page.

I'll prepare a 3.42.1 today. Thanks for catching this, Julien!

Comment 64

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

Assigning CVE-2018-18508 to this.

Comment 65

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

Embargo over; tests landed in bug 1521174.

Comment 66

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

Patch landed in trunk as https://hg.mozilla.org/projects/nss/rev/5e70b72131ac.