1467938 - VP9 Missing Frame Processing Out-of-Bounds Memory Access

Status: RESOLVED FIXED

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

Description

[Nils Ohlmeier [:drno]]

Looks like Firefox 62 is affected by this exploit published today: https://www.exploit-db.com/exploits/44863/

Comment 1

[Nils Ohlmeier [:drno]]

And for added fun a second one https://www.exploit-db.com/exploits/44862/

Comment 2

[Nils Ohlmeier [:drno]]

Google Project Zero reports for these:

https://bugs.chromium.org/p/project-zero/issues/detail?id=1567
https://bugs.chromium.org/p/project-zero/issues/detail?id=1568

I don't have access to the Chromium bug reports to see what the fix looks like. Apparently the fix has been shipped in Chrome 67. But CVE-2018-6129 and CVE-2018-6130 don't appear to be disclosed yet either.

Comment 3

[Nils Ohlmeier [:drno]]

Link to the Chromium bug which probably/hopefully contains the fix https://bugs.chromium.org/p/chromium/issues/detail?id=838402

Comment 4

[Daniel Veditz [:dveditz]]

CVE-2018-6130 appears to be https://bugs.chromium.org/p/chromium/issues/detail?id=838402 for which they landed https://webrtc-review.googlesource.com/c/src/+/74923 on the m67 branch (part of a larger https://webrtc-review.googlesource.com/c/src/+/60260).

They subsequently backed out and re-landed as https://webrtc-review.googlesource.com/c/src/+/76480


CVE-2018-6129 appears to be https://bugs.chromium.org/p/chromium/issues/detail?id=838672 which is the following change: https://webrtc-review.googlesource.com/c/src/+/74921

Comment 5

[Nils Ohlmeier [:drno]]

Attachment: bug1467938_part1.patch

Comment 6

[Nils Ohlmeier [:drno]]

Attachment: bug1467938_part1.patch

This is what really landed in Chrome.

Comment 7

[Nils Ohlmeier [:drno]]

Attachment: bug1467938_part1.patch

Fixed the commit comment

Comment 8

[Nils Ohlmeier [:drno]]

Attachment: bug1467938_part2.patch

This is the patch for https://bugs.chromium.org/p/project-zero/issues/detail?id=1568

Comment 9

[Nils Ohlmeier [:drno]]

Attachment: bug1467938_part1.patch

Fixed the return value.

Comment 10

[Nils Ohlmeier [:drno]]

Attachment: bug1467938_part2.patch

Fixed the logging macro.

Comment 11

[Nils Ohlmeier [:drno]]

Comment on attachment 8984634 [details] [diff] [review]
bug1467938_part1.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch? For non-webrtc experts the hardest part is probably to figure out how to get the payload into the affected code. For the record I disagree with the statements in the chromium bug reports that this requires for someone to answer a call. For people with WebRTC knowledge it's pretty easy to write JS code which gets the payload into the browser with no user interaction. Since the demo exploit code is now openly available I think this makes exploit construction pretty easy.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? I took the title from the public Chromium bugs. We could make it less obvious.

Which older supported branches are affected by this flaw? All branches up to release and ESR60. ESR52 is not affected as it uses an older version of the webrtc.org code.

If not all supported branches, which bug introduced the flaw? Probably the import of webrtc.org 57.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Patches apply cleanly to Beta and ESR60.

How likely is this patch to cause regressions; how much testing does it need? They already got released in Chrome 67. This affects only the VP9 codec, which is typically not used in WebRTC these days. Thus I think it's unlikely to cause regressions.

Comment 12

[Nils Ohlmeier [:drno]]

Comment on attachment 8984635 [details] [diff] [review]
bug1467938_part2.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch? For non-webrtc experts the hardest part is probably to figure out how to get the payload into the affected code. For the record I disagree with the statements in the chromium bug reports that this requires for someone to answer a call. For people with WebRTC knowledge it's pretty easy to write JS code which gets the payload into the browser with no user interaction. Since the demo exploit code is now openly available I think this makes exploit construction pretty easy. Just from the patch itself it's a little less obvious what this protects against. But again the attack description is out there in the public now.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? I took the title from the public Chromium bugs. We could make it less obvious.

Which older supported branches are affected by this flaw? All branches up to release and ESR60. ESR52 is not affected as it uses an older version of the webrtc.org code.

If not all supported branches, which bug introduced the flaw? Probably the import of webrtc.org 57.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Patches apply cleanly to Beta and ESR60.

How likely is this patch to cause regressions; how much testing does it need? They already got released in Chrome 67. This affects only the VP9 codec, which is typically not used in WebRTC these days. Thus I think it's unlikely to cause regressions.

Comment 13

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

Comment on attachment 8984635 [details] [diff] [review]
bug1467938_part2.patch

sec-approval+.
We're going to want to backport patches to affected versions, including ESR60. Please nominate Beta and ESR60 patches.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/44ae071a453f285f841d4c3cc13e0b21427ace92

I landed this as a folded-up patch since it seemed a bit more sensible from our end (i.e. avoiding repetitive commit messages and just backporting some upstream patches touching the same bit of code). As was previously noted, this grafts cleanly to Beta and ESR60 as-landed, so it just needs approval requests ASAP.

Comment 15

[Nils Ohlmeier [:drno]]

Comment on attachment 8984634 [details] [diff] [review]
bug1467938_part1.patch

Note this applies to part1 and part2.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1341285 landed the merge of libwebrtc 57 into Fx 56
[User impact if declined]: Potential crashes of the content process
[Is this code covered by automated tests?]: We have automated tests using the VP9 codec. But it is not guaranteed that these tests will cover exactly the modified code here.
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: Not really.
[Why is the change risky/not risky?]: It only adds extra safety checks to catch malformed packets. It has been in Chrome 67 for some time already. And it only applies to the not widely used VP9 codec.
[String changes made/needed]: N/A

Comment 16

[Nils Ohlmeier [:drno]]

Comment on attachment 8984635 [details] [diff] [review]
bug1467938_part2.patch

See request for part1

Comment 17

[Nils Ohlmeier [:drno]]

Comment on attachment 8984634 [details] [diff] [review]
bug1467938_part1.patch

Applies to part1 and part2.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: It is sec-high
User impact if declined: crashes of content process 
Fix Landed on Version: 62
Risk to taking this patch (and alternatives if risky): It is not very risky (see explanation in Beta request).
String or UUID changes made by this patch: N/A

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 18

[Nils Ohlmeier [:drno]]

Comment on attachment 8984635 [details] [diff] [review]
bug1467938_part2.patch

See request for part 1

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/44ae071a453f

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8984634 [details] [diff] [review]
bug1467938_part1.patch

Approved for 61.0b13 and ESR 60.1.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bd79850d10f8
https://hg.mozilla.org/releases/mozilla-esr60/rev/9875e9b3e0ca