1683964 - (CVE-2020-16044) Use-after-free write when handling malicious COOKIE-ECHO

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P1)

Description

[Andrew McCreight [:mccr8]]

I don't know if this applies to Firefox or not, but Project Zero just opened up a bug containing a use-after-free in SCTP: https://bugs.chromium.org/p/project-zero/issues/detail?id=2094

The commit said to fix it is here: https://github.com/sctplab/usrsctp/commit/d844a690d30ace22de75e8b80d9bb08ae7283795

I didn't check the whole patch, but our code in netwerk/sctp/src/netinet/sctp_input.c at least appears to match the pre-patch state, and neither that file nor netwerk/sctp/src/netinet/sctp_pcb.c have changed since July.

Comment 1

[Andrew McCreight [:mccr8]]

(the bug summary I used just comes from the Project Zero report)

Comment 2

[Andrew McCreight [:mccr8]]

I guess the patch in comment 0 has been public since September, for what it is worth.

Comment 3

[Andrew McCreight [:mccr8]]

Oops, didn't mean to do that needinfo.

Comment 4

[Andrew McCreight [:mccr8]]

It looks like Chrome is not shipping this fix either. tjr found this history log where they apparently landed it as part of a large patch rollup, but backed it out because something in there was causing issues: https://chromium.googlesource.com/chromium/src/+log/HEAD/third_party/usrsctp (this page takes a while to load)

Comment 5

[Nils Ohlmeier [:drno]]

(In reply to Andrew McCreight [:mccr8] from comment #4)

It looks like Chrome is not shipping this fix either. tjr found this history log where they apparently landed it as part of a large patch rollup, but backed it out because something in there was causing issues: https://chromium.googlesource.com/chromium/src/+log/HEAD/third_party/usrsctp (this page takes a while to load)

Yes in https://bugs.chromium.org/p/chromium/issues/detail?id=1157658 they reverted a big rollup patch which included the security fix, because Chrome had picked up changes in libusrsctp which was trying to use getrandom() without checking for it's availability.
Our version of libusrsctp does appear to have any calls to getrandom() in it yet. So I assume it should be save to cherry pick just the security fix.

Comment 6

[Nils Ohlmeier [:drno]]

Attachment: Bug 1683964: Improve the input validation and processing of sctp cookies.

Comment 7

[Tom Ritter [:tjr]]

Rating this -critical because P0 has publicly disclosed the bug and they get a lot of attention. But Chrome hasn't shipped the fix yet; which is odd.

Comment 8

[Nils Ohlmeier [:drno]]

Turns out that https://github.com/sctplab/usrsctp/commit/d844a690d30ace22de75e8b80d9bb08ae7283795?branch=d844a690d30ace22de75e8b80d9bb08ae7283795&diff=split was only a part of series of problems P0 found in how usrsctp handles cookies.
The usrsctp maintainers recommended to take these two patches as well:
https://github.com/sctplab/usrsctp/commit/355fb576b1a9dfef70fc0e158d66692662baaddc
https://github.com/sctplab/usrsctp/commit/7dab23aa0d8db86fedd222a308067439b03fad0f

I'll add them to the patch shortly.

Comment 9

[Nils Ohlmeier [:drno]]

Attachment: Bug 1683964: clean up more resources of existing SCTP association in case of a restart.

Depends on D100379

Comment 10

[Nils Ohlmeier [:drno]]

Attachment: Bug 1683964: Harden the handling of outgoing streams.

Depends on D100490

Comment 11

[Tom Ritter [:tjr]]

Comment on attachment 9194513 [details]
Bug 1683964: Improve the input validation and processing of sctp cookies.

pre-emptively sec-approving these, they are already public in several ways.

Comment 12

[Tom Ritter [:tjr]]

Comment on attachment 9194766 [details]
Bug 1683964: clean up more resources of existing SCTP association in case of a restart.

pre-emptively sec-approving these, they are already public in several ways.

Comment 13

[Tom Ritter [:tjr]]

Comment on attachment 9194767 [details]
Bug 1683964: Harden the handling of outgoing streams.

pre-emptively sec-approving these, they are already public in several ways.

Comment 14

[Nils Ohlmeier [:drno]]

Comment on attachment 9194513 [details]
Bug 1683964: Improve the input validation and processing of sctp cookies.

Security Approval Request (just for the record):

• How easily could an exploit be constructed based on the patch?: Patches are already publicly available upstream. Probably requires SCTP protocol expertise to figure out how to abuse it.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No (but the patches have landed upstream in public already).
• Which older supported branches are affected by this flaw?: All
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: I'm not really certain about this. These changes are deep in the SCTP protocol levels and could cause problems. But they have been reviewed and merged upstream, and are not known for causing any problems so far.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/08ba03dc8d4420e04e7c77fee3013e68180e6ead
https://hg.mozilla.org/integration/autoland/rev/8c09f4813fc7e8f44605b6092262199bff15cdd7
https://hg.mozilla.org/integration/autoland/rev/5991645a87d2abf289686d09d943229c9e3e54b5

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/08ba03dc8d44
https://hg.mozilla.org/mozilla-central/rev/8c09f4813fc7
https://hg.mozilla.org/mozilla-central/rev/5991645a87d2

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Please request uplift to beta, release and esr78.

Comment 18

[Nils Ohlmeier [:drno]]

Comment on attachment 9194513 [details]
Bug 1683964: Improve the input validation and processing of sctp cookies.

Beta/Release Uplift Approval Request

• User impact if declined: Attacker can potentially break the SCTP cookie and create a use after free in Firefox.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Patches have been merged upstream for several weeks now with no issues reported so far.
• String changes made/needed: N/A

Comment 19

[Nils Ohlmeier [:drno]]

Comment on attachment 9194513 [details]
Bug 1683964: Improve the input validation and processing of sctp cookies.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-crit
• User impact if declined: Attacker can potentially break the SCTP cookie and create a use after free in Firefox.
• Fix Landed on Version: 86
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Patches have been merged upstream for several weeks now with no issues reported so far.
• String or UUID changes made by this patch: N/A

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9194513 [details]
Bug 1683964: Improve the input validation and processing of sctp cookies.

Approved for 85.0b5, 84.0.2, and 78.6.1esr. And Fenix 84.1.3 and Focus 8.11.3.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a3eca32a9347
https://hg.mozilla.org/releases/mozilla-beta/rev/17c1ed4ac8f7
https://hg.mozilla.org/releases/mozilla-beta/rev/4d57860f99fd

https://hg.mozilla.org/releases/mozilla-release/rev/f927be5421eb
https://hg.mozilla.org/releases/mozilla-release/rev/ea718a2776ca
https://hg.mozilla.org/releases/mozilla-release/rev/7e22d68e1ebf

Comment 22

[Ryan VanderMeulen [:RyanVM]]

78.6.1esr
https://hg.mozilla.org/releases/mozilla-esr78/rev/4158db020f68
https://hg.mozilla.org/releases/mozilla-esr78/rev/a3ea1c463e0e
https://hg.mozilla.org/releases/mozilla-esr78/rev/df0581e37d87

78.7.0esr+
https://hg.mozilla.org/releases/mozilla-esr78/rev/ed7b601f84c6
https://hg.mozilla.org/releases/mozilla-esr78/rev/9f369d3648f8
https://hg.mozilla.org/releases/mozilla-esr78/rev/d558b841e2f3

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 24

[Nils Ohlmeier [:drno]]

Declaring NI bankruptcy.