Closed
Bug 1273202
Opened 8 years ago
Closed 8 years ago
HTMLInputElement may dispatch events inside its destructor
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
People
(Reporter: smaug, Assigned: smaug)
Details
(Keywords: sec-high, Whiteboard: [post-critsmash-triage][adv-main47+][adv-esr45.2+])
Attachments
(2 files)
2.64 KB,
patch
|
jwatt
:
review+
|
Details | Diff | Splinter Review |
2.83 KB,
patch
|
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
ritu
:
approval-mozilla-esr45+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
This should fix bug 1210595.
Attachment #8752942 -
Flags: review?(jwatt)
Assignee | ||
Comment 1•8 years ago
|
||
This is sec-high or -critical, since we end up touching deleted objects.
Keywords: sec-high
Comment 2•8 years ago
|
||
Comment on attachment 8752942 [details] [diff] [review] patch Review of attachment 8752942 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the enum added. ::: dom/html/HTMLInputElement.cpp @@ +1006,5 @@ > > HTMLInputElement::~HTMLInputElement() > { > if (mNumberControlSpinnerIsSpinning) { > + StopNumberControlSpinnerSpin(false); Ugh @ bool arg. This looks like the call is saying "don't stop spinner spins". Please add an enum value for this.
Attachment #8752942 -
Flags: review?(jwatt) → review+
Assignee | ||
Comment 3•8 years ago
|
||
makes probably a bit harder to merge this to all the branches, but sure.
Assignee | ||
Comment 4•8 years ago
|
||
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8752959 [details] [diff] [review] with enum [Security approval request comment] How easily could an exploit be constructed based on the patch? Hmm, maybe some JS could access the dispatched event and then afterwards do something with the .target field which internally would point to deleted object... Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? eDisallowDispatchingEvents in dtor does hint pretty strongly where the issue is, but how to actually utilize that...not clear. Which older supported branches are affected by this flaw? all Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The patch seems to apply to beta tree with some fuzz. How likely is this patch to cause regressions; how much testing does it need? Should be very safe. Effectively adding an if-check to prevent doing the bad stuff during dtor call.
Attachment #8752959 -
Flags: sec-approval?
Attachment #8752959 -
Flags: approval-mozilla-esr45?
Attachment #8752959 -
Flags: approval-mozilla-beta?
Attachment #8752959 -
Flags: approval-mozilla-aurora?
Comment 6•8 years ago
|
||
sec-approval+ for trunk. I'd like to see this in Aurora, Beta, and ESR45 as well as a sec-high JS issue.
status-firefox46:
--- → wontfix
status-firefox47:
--- → affected
status-firefox48:
--- → affected
status-firefox49:
--- → affected
status-firefox-esr45:
--- → affected
tracking-firefox47:
--- → ?
tracking-firefox48:
--- → +
tracking-firefox49:
--- → +
tracking-firefox-esr45:
--- → 47+
Updated•8 years ago
|
Attachment #8752959 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/87e32446023f9d56c5ca367834b1d9ee7a1701c3
https://hg.mozilla.org/mozilla-central/rev/87e32446023f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8752959 [details] [diff] [review] with enum Sec-high, Aurora48+, Beta47+, ESR45+
Attachment #8752959 -
Flags: approval-mozilla-esr45?
Attachment #8752959 -
Flags: approval-mozilla-esr45+
Attachment #8752959 -
Flags: approval-mozilla-beta?
Attachment #8752959 -
Flags: approval-mozilla-beta+
Attachment #8752959 -
Flags: approval-mozilla-aurora?
Attachment #8752959 -
Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/9942206d3f9c https://hg.mozilla.org/releases/mozilla-beta/rev/0723a0212f5e https://hg.mozilla.org/releases/mozilla-esr45/rev/b74f1ab939d2
Updated•8 years ago
|
Group: core-security-release
Updated•8 years ago
|
Group: dom-core-security
Updated•8 years ago
|
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Updated•8 years ago
|
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main47+][adv-esr45.2+]
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•