Christoph Diehl [:posidron] Reporter
	Description • 6 years ago

From Google's Project Zero:

WebRTC: Use-after-free in VP8 Block Decoding
https://bugs.chromium.org/p/project-zero/issues/detail?id=1575

	Nils Ohlmeier [:drno]
	Comment 1 • 6 years ago

Again the same situation: Project Zero discloses and the Chromium bug is still not accessible.

	Nils Ohlmeier [:drno]
	Comment 2 • 6 years ago

Dan can you check if you can find out the commit which fixed this upstream?

	Daniel Veditz [:dveditz]
	Comment 3 • 6 years ago

Given the chrome bug # given in the GPZ bug it should be this one:
https://chromium.googlesource.com/webm/libvpx/+/52add5896661d186dec284ed646a4b33b607d2c7

	Nils Ohlmeier [:drno]
	Comment 4 • 6 years ago

That looks very plausible. Dan can you look into cherry picking that fix into our affected versions?

	Dan Minor [:dminor] Assignee
	Comment 5 • 6 years ago

Since this is fixed on head and not the v1.7.0 release, we'll need to carry
the patch until the next release of libvpx.

	Dan Minor [:dminor] Assignee
	Comment 6 • 6 years ago

Sure thing.

	Dan Minor [:dminor] Assignee
	Comment 7 • 6 years ago

Comment on attachment 8998165 [details]
Bug 1480092 - Cherrypick rev 52add5896661d186dec284ed646a4b33b607d2c7; r=drno

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easily.

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 upstream patch mentions that it fixes a UAF.

Which older supported branches are affected by this flaw?
All.

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
No, but easy to create, this is a one liner.

How likely is this patch to cause regressions; how much testing does it need?
Unlikely, patch has already in use upstream.

	Dan Minor [:dminor] Assignee
	Comment 8 • 6 years ago

:drno, the review in phabricator says I need to make revisions to proceed. Based on your LGTM, I assume you meant to leave a r+ but something went wrong. Please let me know.

	Nils Ohlmeier [:drno]
	Comment 9 • 6 years ago

(In reply to Dan Minor [:dminor] from comment #8)
> :drno, the review in phabricator says I need to make revisions to proceed.
> Based on your LGTM, I assume you meant to leave a r+ but something went
> wrong. Please let me know.

Thanks for pinging me. That was pretty clearly a UI bug in phabricator. I did select "accept" and then typed my comment. Apparently typing my comment reset the drop down to the default value :-(

	Nils Ohlmeier [:drno]
	Comment 10 • 6 years ago

Comment on attachment 8998165 [details]
Bug 1480092 - Cherrypick rev 52add5896661d186dec284ed646a4b33b607d2c7; r=drno

Nils Ohlmeier [:drno] has approved the revision.

	Al Billings [:abillings - ex-MoCo]
	Comment 11 • 6 years ago

Comment on attachment 8998165 [details]
Bug 1480092 - Cherrypick rev 52add5896661d186dec284ed646a4b33b607d2c7; r=drno

sec-approval+ for trunk. We'll want Beta and ESR60 patches nominated too.

	Dan Minor [:dminor] Assignee
	Comment 12 • 6 years ago

Comment on attachment 8998165 [details]
Bug 1480092 - Cherrypick rev 52add5896661d186dec284ed646a4b33b607d2c7; r=drno

Approval Request Comment
[Feature/Bug causing the regression]:
This is a bug in an upstream library.
[User impact if declined]:
Security problems.
[Is this code covered by automated tests?]:
No.
[Has the fix been verified in Nightly?]:
No, but this is a patch from upstream which has been verified in Chrome.
[Needs manual test from QE? If yes, steps to reproduce]:
No. 
[List of other uplifts needed for the feature/fix]:
None.
[Is the change risky?]:
No.
[Why is the change risky/not risky?]:
This is a cherrypick of an upstream patch.
[String changes made/needed]:
None.

	Dan Minor [:dminor] Assignee
	Comment 13 • 6 years ago

Comment on attachment 8998165 [details]
Bug 1480092 - Cherrypick rev 52add5896661d186dec284ed646a4b33b607d2c7; r=drno

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

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

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 6 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/16aa20e2e300fc82772935add7a61d2004c081eb

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 15 • 6 years ago

https://hg.mozilla.org/mozilla-central/rev/16aa20e2e300

	Ryan VanderMeulen [:RyanVM]
	Comment 16 • 6 years ago

Comment on attachment 8998165 [details]
Bug 1480092 - Cherrypick rev 52add5896661d186dec284ed646a4b33b607d2c7; r=drno

Fixes a sec-high. Approved for 62.0b18 and ESR 60.2.

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 17 • 6 years ago
uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/462f6b16d588

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 6 years ago
uplift

https://hg.mozilla.org/releases/mozilla-esr60/rev/01a8adcbeab6