1111079 - Chromium IPC channel bug: use-after-free in IPC::Channel::ChannelImpl::ProcessOutgoingMessages()

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Jed Davis [:jld] (away until 10-15)]

Upstream bug report: https://crbug.com/22451

Comment 1

[Andrew McCreight [:mccr8]]

Is this something you can look at soon, Jed?

Comment 2

[Matt Wobensmith [:mwobensmith][:matt:]]

Hi Jed, any ideas?

Comment 3

[Jed Davis [:jld] (away until 10-15)]

Attachment: Patch

Here's a patch, but I don't have any test cases to verify that it actually fixes anything.

Mildly obscured try run, combined with bug 1111065: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea158edc4d70

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8578024 [details] [diff] [review]
Patch

Review of attachment 8578024 [details] [diff] [review]:
-----------------------------------------------------------------

Looks sane to me.

Comment 5

[Jed Davis [:jld] (away until 10-15)]

Comment on attachment 8578024 [details] [diff] [review]
Patch

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

I'm not sure.  I'm not even sure how exploitable this is.  The upstream bug mentions a Chromium test case that caused valgrind failures, but it doesn't correspond directly to our use of IPC.  Someone who knows the IPC code better might have a better idea.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Probably.  I can remove the upstream commit ids from the commit message.  The code comments, I'd prefer to leave intact.

But this bug has been public upstream since 2012: https://crbug.com/22451#c25

> Which older supported branches are affected by this flaw?

Potentially all of them, but see above about how I don't actually know what the impact is.  Even branches that don't use IPC as a security boundary might be affected, if it were possible for web content to somehow cause the use-after-free to occur.

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

No, but not difficult; anything older than 35 will have a merge conflict with bug 1063318 but it's trivial to resolve.

> How likely is this patch to cause regressions; how much testing does it need?

Someone with a better understanding of the IPC code might have a better idea about this.

Comment 6

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

Comment on attachment 8578024 [details] [diff] [review]
Patch

sec-approval+ for trunk. We will want beta and aurora patches for this once it is in.

I don't think we'll take this for ESR31 as it isn't clear it is affected by the issue.

Comment 7

[Lawrence Mandel [:lmandel] (use needinfo)]

The final 37 Beta goes to build on Thu (Mar 19). Can you please create a Beta branch patch if one is necessary and request uplift approval by tomorrow morning?

Comment 8

[Jed Davis [:jld] (away until 10-15)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e01fd5b164d6

Comment 9

[Jed Davis [:jld] (away until 10-15)]

Attachment: bug1111079-hg2.diff

Approval Request Comment
[Feature/regressing bug #]: GeckoMediaPlugin sandboxing
[User impact if declined]: See earlier comments about security implications.
[Describe test coverage new/current, TreeHerder]: Built and tested dom/media/test/test_eme_playback.html on Linux/x86_64 on aurora and beta.  See also previous m-c try run.
[Risks and why]: Some; this touches core IPC code.
[String/UUID change made/needed]: None.


Note: I edited the commit message before landing (to omit the pointers to specific Chromium issues, and give it a description that may or may not be innocuous enough), which is reflected here; otherwise it's the same as the previous patch.

Comment 10

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8580350 [details] [diff] [review]
bug1111079-hg2.diff

This has already landed on inbound so we should get it onto Beta and Aurora. Beta+ Aurora+

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/276d869dd9e5
https://hg.mozilla.org/releases/mozilla-beta/rev/5bb1bb65cc28
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/bf15248ee754
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c025034f7323
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/5dc250261d6a

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e01fd5b164d6

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5bb1bb65cc28
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/bf15248ee754
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/c025034f7323