Closed Bug 1021969 Opened 10 years ago Closed 10 years ago

mozMediaStatisticsShowing property shouldn't be content-exposed

Categories

(Toolkit :: Video/Audio Controls, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla32
Tracking Status
firefox29 --- wontfix
firefox30 --- wontfix
firefox31 --- verified
firefox32 --- verified
firefox-esr24 --- wontfix

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

(Keywords: sec-low, Whiteboard: p=2 s=33.1 [adv-main31+])

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1021240 +++

1. Create a video element with controls = true
2. Set the getter for the 'mozMediaStatisticsShowing' property to a function that calls alert
3. Add the video element to the dom tree.
4. Firefox freeze


We'll work around this by avoiding the mozMediaStatisticsShowing property from influencing chrome by sticking it in a webidl property.
Landed

https://bugzilla.mozilla.org/attachment.cgi?id=8435888

as 

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/9e19217a1042

Marco, can you add this to the current iteration?
Status: NEW → ASSIGNED
Flags: needinfo?(mmucci)
Flags: firefox-backlog+
Whiteboard: [fixed-in-inbound] p=2 s=it-32c-31a-30b.3 [qa-]
Added to Iteration 32.3
Flags: needinfo?(mmucci)
https://hg.mozilla.org/mozilla-central/rev/9e19217a1042
https://hg.mozilla.org/mozilla-central/rev/115ea0cc4b22

Should this have had sec-approval to land?
https://wiki.mozilla.org/Security/Bug_Approval_Process
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: needinfo?(gijskruitbosch+bugs)
Resolution: --- → FIXED
Whiteboard: [fixed-in-inbound] p=2 s=it-32c-31a-30b.3 [qa-] → p=2 s=it-32c-31a-30b.3 [qa-]
Target Milestone: --- → mozilla32
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #4)
> https://hg.mozilla.org/mozilla-central/rev/9e19217a1042
> https://hg.mozilla.org/mozilla-central/rev/115ea0cc4b22
> 
> Should this have had sec-approval to land?
> https://wiki.mozilla.org/Security/Bug_Approval_Process

Presumably yes. I'm sorry; I actually quite literally didn't realize we had process for landing sec fixes on m-c/nightly-integration. I /believe/ this wasn't sec-high or sec-crit, but I'm needinfo'ing bholley and dveditz to be sure. I also believe it is a well-known fact that XPCN.unwrap should be avoided, and so I don't think the patch really gives any information that any people looking for exploits wouldn't have already by applying simple grepping to our source.

I also think that this could easily and therefore should be uplifted to 31. I don't think it's a severe enough issue that it warrants a respin of 30 rc.

The original issue is a hang (OSX-only, I *think* ?), which is being tracked in bug 1021240. I checked when writing the patch and wasn't able to abuse the exposed unwrapped flag into privilege escalation or something of the sort, but I'm not exactly a security researcher so that's hardly a guarantee that there genuinely isn't such a possibility.
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(dveditz)
Flags: needinfo?(bobbyholley)
Status: RESOLVED → VERIFIED
I think this was a sec:low, personally.
Definitely a sec-low as it stands.  Put all the original issues together and it might manage sec-moderate.
Flags: needinfo?(dveditz)
Flags: needinfo?(bobbyholley)
Keywords: sec-low
Attached patch Branch patchSplinter Review
Patch for 31
Comment on attachment 8436897 [details] [diff] [review]
Branch patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: freeze when website defines getter calling alert() for mozMediaStatisticsShowing
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #8436897 - Flags: review+
Attachment #8436897 - Flags: approval-mozilla-beta?
Attachment #8436897 - Flags: approval-mozilla-aurora?
(to be clear, requesting approval for 31, not for 30)
Comment on attachment 8436897 [details] [diff] [review]
Branch patch

Approving for 31. Leaving the "?" for beta in case it does not reach aurora before the merge.
Attachment #8436897 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hi Marco - you marked the bug verified, but the flags for Fx31/Fx32 remain unchanged. Has the bug actually been verified fixed by someone other than the developer? If so, could you set those flags to indicate which branches you verified on? 

QA relies on these flags to know what security bugs need verification, and the state of this bug is unclear. Thank you very much.
Flags: needinfo?(mmucci)
(In reply to Matt Wobensmith from comment #13)
> Hi Marco - you marked the bug verified, but the flags for Fx31/Fx32 remain
> unchanged. Has the bug actually been verified fixed by someone other than
> the developer? If so, could you set those flags to indicate which branches
> you verified on? 
> 
> QA relies on these flags to know what security bugs need verification, and
> the state of this bug is unclear. Thank you very much.

Hi Matt, the bug was marked as [qa-] as not required further QA verification.  That's why I changed the status to verified after it was resolved.  If this is not the case and the bug requires further verification I can mark it as [qa+] and change the status back to resolved.  Let me know what is the best way to proceed for this particular bug.
Flags: needinfo?(mmucci)
I set this to qa-, and thinking about it more, that's probably a mistake.

Matt, you can verify that this fixes the freeze with the testcase at https://bugzilla.mozilla.org/attachment.cgi?id=8435220 (from bug 1021240).
Status: VERIFIED → RESOLVED
Closed: 10 years ago10 years ago
QA Contact: mwobensmith
Whiteboard: p=2 s=it-32c-31a-30b.3 [qa-] → p=2 s=it-32c-31a-30b.3 [qa+]
Added to Iteration 33.1
Whiteboard: p=2 s=it-32c-31a-30b.3 [qa+] → p=2 s=33.1 [qa+]
Thanks Marco and Gijs for clearing up that mystery. :)

Confirmed issue on Fx31, 2014-06-05.
Verified fixed on Fx31 and Fx32, 2014-06-16.
Status: RESOLVED → VERIFIED
Whiteboard: p=2 s=33.1 [qa+] → p=2 s=33.1
Whiteboard: p=2 s=33.1 → p=2 s=33.1 [adv-main31+]
Alias: CVE-2014-1554
Alias: CVE-2014-1554
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.