Closed
Bug 1359849
Opened 8 years ago
Closed 8 years ago
videocontrols needs to try/catch around access to all content bits
Categories
(Core :: Audio/Video, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla55
People
(Reporter: bzbarsky, Assigned: jaws)
Details
(Keywords: sec-other, Whiteboard: [adv-main55-][post-critsmash-triage])
Attachments
(1 file)
|
3.21 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
Bug 1356558 describes a situation (see bug 1356558 comment 0 and bug 1356558 comment 20) in which videocontrols code throws an exception due to the object it's trying to touch having been hueyfixed and hence a dead object wrapper.
Unfortunately, that means videocontrols don't finish the cleanup they are trying to do, causing page-lifetime leaks. There should probably be try/catches around this stuff.
Security-sensitive for now, because bug 1356558 is.
Ray, Jared, do you know who owns this stuff? It looks like you touched/reviewed it recently...
Flags: needinfo?(ralin)
Flags: needinfo?(jaws)
|
Reporter | |
Comment 1•8 years ago
|
||
> and bug 1356558 comment 20
I meant bug 1356558 comment 21.
|
Assignee | |
Updated•8 years ago
|
Flags: needinfo?(jaws)
|
|
Assignee | |
Updated•8 years ago
|
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Flags: needinfo?(ralin)
|
|
Assignee | |
Comment 2•8 years ago
|
||
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #0)
> Ray, Jared, do you know who owns this stuff? It looks like you
> touched/reviewed it recently...
That would be me, working on a patch now.
|
|
Assignee | |
Comment 3•8 years ago
|
||
Attachment #8862045 -
Flags: review?(gijskruitbosch+bugs)
|
||
Comment 4•8 years ago
|
||
Comment on attachment 8862045 [details] [diff] [review]
1359849.patch
Review of attachment 8862045 [details] [diff] [review]:
-----------------------------------------------------------------
Does this need a try...catch too:
https://dxr.mozilla.org/mozilla-central/rev/f229b7e5d91eb70d23d3e31db7caff9d69a2ef04/toolkit/content/widgets/videocontrols.xml#2041
?
Attachment #8862045 -
Flags: review?(gijskruitbosch+bugs) → review+
|
|
Assignee | |
Comment 5•8 years ago
|
||
(In reply to :Gijs from comment #4)
> Does this need a try...catch too?
Wouldn't hurt. I added one there and will land this on inbound once the tree reopens.
|
|
Assignee | |
Comment 6•8 years ago
|
||
|
||
Comment 8•8 years ago
|
||
Actually, looks like it goes back way further than that even.
status-firefox53:
--- → wontfix
status-firefox54:
--- → affected
status-firefox55:
--- → affected
status-firefox-esr45:
--- → wontfix
status-firefox-esr52:
--- → affected
|
|
Assignee | |
Comment 9•8 years ago
|
||
This goes back to at least Firefox 13, http://searchfox.org/mozilla-central/rev/f6573cb36c7c82c25c3b8e14ed2ee698bd32ee59/toolkit/content/widgets/videocontrols.xml#639
Flags: needinfo?(jaws)
|
||
Comment 10•8 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
|
||
Updated•8 years ago
|
Group: dom-core-security → core-security-release
|
|
||
Comment 11•8 years ago
|
||
Can this ride the 55 train or should we consider it for Beta/ESR52 uplift?
Flags: needinfo?(jaws)
|
|
Reporter | |
Comment 12•8 years ago
|
||
I wouldn't worry about uplifting this one. It's really only sec-sensitive because bug 1356558 is.
|
|
||
Updated•8 years ago
|
|
||
Updated•8 years ago
|
Whiteboard: [adv-main55-]
|
||
Updated•8 years ago
|
Flags: qe-verify-
Whiteboard: [adv-main55-] → [adv-main55-][post-critsmash-triage]
|
|
||
Updated•7 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•