1062981 - (CVE-2014-1586) Navigating away from a page with camera sharing in an iframe leaves camera recording

Status: VERIFIED FIXED

(Core :: WebRTC, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

+++ This bug was initially created as a clone of another bug +++

STR:

1. Go to http://mozilla.github.com/webrtc-landing/gum_iframe.html
2. Click video to enable camera sharing in one of the iframes.
3. Navigate away (type in any URL) and observe that the camera light stays on.
4. Hit the back button, and there you are, still being recorded it seems, visuals and all.

The same problem exists if you navigate away using the Back button in step 3.

This problem seems specific to camera sharing in iframes. I've verified that MediaManager::OnNavigation is not called in these cases.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Disable bfcache for pages with active gUM

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8484653 [details] [diff] [review]
Disable bfcache for pages with active gUM

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

::: content/base/src/nsDocument.cpp
@@ +8507,5 @@
>    }
>  
> +#ifdef MOZ_MEDIA_NAVIGATOR
> +  // Check if we have active GetUserMedia use
> +  MediaManager* mediaManager = MediaManager::Get();

This forces it to create the MediaManager on the first navigation.  You need to be able to check for it without forcing it to be created (and the MediaManager thread!)

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Disable bfcache for pages with active gUM (2)

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8484709 [details] [diff] [review]
Disable bfcache for pages with active gUM (2)

>+  static bool Exists() {
{ goes to its own line.

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Disable bfcache for pages with active gUM (3) r=smaug, jesup

Incorporated feedback. Carrying forward r=smaug,jesup

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8485182 [details] [diff] [review]
Disable bfcache for pages with active gUM (3) r=smaug, jesup

[Security approval request comment]
How easily could an exploit be constructed based on the patch? moderately easy (if you realize it's related to iframes).  Note that the severity isn't super-high, though it's confusing.  Pages in the bfcache don't run, but the camera (and light if any) remain on.  Navigating back to the page re-activates the (already-permitted-by-the-user) capture, which is confusing.  Also confuses unanswered requests.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? somewhat, but no mention of iframes.

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? trivial backport

How likely is this patch to cause regressions; how much testing does it need?  Very low chance of regressions.

Comment 7

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

Is this a sec-high or critical rated issue? We should get a rating on this before sec-approval.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

I'd suggest sec-moderate for this one, per the comments above.  Tends to confuse more than anything.

Comment 9

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

Comment on attachment 8485182 [details] [diff] [review]
Disable bfcache for pages with active gUM (3) r=smaug, jesup

Making this a sec-moderate then. As such, it can land at will and doesn't need sec-approval.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6377d0124222

I believe this affects all revs; as a sec-moderate with a low-risk patch we should consider how much uplifting to do.  Aurora very likely, beta probably, esr31 probably, 2.1 (which is aurora).  I probably wouldn't uplift to 2.0 or release/32, though those are possible.  On b2g it may be less obvious that the camera is active if you navigate back to a page (though this hasn't been confirmed to work on b2g yet).

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6377d0124222

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Please request aurora/beta approval on this when you get a chance :)

Comment 13

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8485182 [details] [diff] [review]
Disable bfcache for pages with active gUM (3) r=smaug, jesup

Thanks the reminder.

Approval Request Comment
[Feature/regressing bug #]: gUM
[User impact if declined]:
Pages with gUM in iframes leave the camera running when navigating away from the page. While pages in the bfcache don't actually run, the camera (and light if any) remain on.  Navigating back to the page re-activates the (already-permitted-by-the-user) capture, which is confusing and gives the impression that recording has continued for the duration.  Additionally, any unanswered permission requests get confused by this, and a page refresh is needed to bring UX components back in sync.

[Describe test coverage new/current, TBPL]: Verified locally. Landed on m-c.
[Risks and why]: 
Very low risk. Returns false on gUM activity in window in bfcache-eligibility test.
[String/UUID change made/needed]: none

Comment 14

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8485182 [details] [diff] [review]
Disable bfcache for pages with active gUM (3) r=smaug, jesup

Approval Request Comment
[Feature/regressing bug #]: gUM
[User impact if declined]:
Pages with gUM in iframes leave the camera running when navigating away from the page. While pages in the bfcache don't actually run, the camera (and light if any) remain on.  Navigating back to the page re-activates the (already-permitted-by-the-user) capture, which is confusing and gives the impression that recording has continued for the duration.  Additionally, any unanswered permission requests get confused by this, and a page refresh is needed to bring UX components back in sync.

[Describe test coverage new/current, TBPL]: Verified locally. Landed on m-c and aurora.
[Risks and why]: 
Very low risk. Returns false on gUM activity in window in bfcache-eligibility test.
[String/UUID change made/needed]: none

Comment 15

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

Comment on attachment 8485182 [details] [diff] [review]
Disable bfcache for pages with active gUM (3) r=smaug, jesup

Beta+

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0a4f8c5845d3
https://hg.mozilla.org/releases/mozilla-beta/rev/46abad0899f9

Comment 17

[:Gavin Sharp [email: [e-mail]]]

Better for users to know this bug exists, hiding it doesn't meaningfully help avoid it being exploited.

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

Agreed. The risk from this bug is at most moderate since the page is frozen in the bfcache.

Comment 19

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue from comment #0 using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/09/2014-09-04-03-02-02-mozilla-central/

Went through the following verification:

fx35:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-10-07-03-02-02-mozilla-central/
- Win 8.1 x64 - PASSED 
- OSX 10.9.5 x64 - PASSED

fx34:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-10-07-00-40-03-mozilla-aurora/
- Win 8.1 x64 - PASSED
- OSX 10.9.5 x64 - PASSED

fx33:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/33.0-candidates/build1/
- Win 8.1 x64 - PASSED
- OSX 10.9.5 x64 - PASSED

Test Cases Used:

- ensured that the camera light is turned off when navigating to another page
- ensured that returning doesn't automatically re-enable the camera
- ensured that the camera sharing icon isn't left behind when navigating away from the WebRTC page
- ensured that the camera light is turned off when closing the active tab

Comment 20

[Daniel Veditz [:dveditz]]

[Tracking Requested - why for this release]:

Not sure why Ryan wontfixed this for ESR31 in comment 12, but related bug 1062876 /was/ fixed on ESR31 so now we have the overall issue half-fixed on that branch. This is a small patch. On the other hand it's late and the other bug fixed the worst aspects. Since there will be a security advisory this oversight isn't likely to get lost in the noise.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

(In reply to Daniel Veditz [:dveditz] from comment #20)

Sorry, I'd been led to believe that typically sec-moderate is wontfix for esr.

Comment 22

[Daniel Veditz [:dveditz]]

Comment on attachment 8485182 [details] [diff] [review]
Disable bfcache for pages with active gUM (3) r=smaug, jesup

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: privacy bug partially fixed
Fix Landed on Version: Firefox 33
Risk to taking this patch (and alternatives if risky): Low (alternative, wait for 31.3 and explain why we only half-fixed issues found as part of bug 1062876)
String or UUID changes made by this patch: none

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

Comment 23

[Daniel Veditz [:dveditz]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #21)
> Sorry, I'd been led to believe that typically sec-moderate is wontfix for
> esr.

Typically, yes, but not absolute though. Please leave for the security team.

Comment 24

[Daniel Veditz [:dveditz]]

Comment on attachment 8485182 [details] [diff] [review]
Disable bfcache for pages with active gUM (3) r=smaug, jesup

a+ for ESR31 (interpreting lmandel's non-objection as approval: "Can you land today and work with bkerensa for approval and ensuring that this is in the rebuild?")

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr31/rev/833e364d63db

Comment 26

[Kamil Jozwiak [:kjozwiak]]

Went through verification using the following build:
- https://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/31.2.0esr-candidates/build3/

I went through the same test cases from comment #19 without any issues:
- OSX 10.9.5 x64 - PASSED
- Win 8.1 x64 - PASSED