Closed Bug 877910 Opened 7 years ago Closed 7 years ago

opening railsdiff crashes Firefox in mozilla::dom::HTMLOptionsCollection::NamedItem @ JSAutoCompartment::JSAutoCompartment

Categories

(Core :: DOM: Core & HTML, defect, critical)

23 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox22 --- unaffected
firefox23 + fixed
firefox24 --- fixed

People

(Reporter: soeren.hentzschel, Assigned: Ms2ger)

References

()

Details

(Keywords: crash, regression, reproducible)

Crash Data

Attachments

(2 files, 1 obsolete file)

STR:

1. open http://railsdiff.org/html/v3.2.13-v4.0.0.rc1.html with Nightly or Aurora and enabled JavaScript

Result:

Firefox crashes

Crash Signature:
[@ JSAutoCompartment::JSAutoCompartment(JSContext*, JSObject*) ]

Crash Report:
https://crash-stats.mozilla.com/report/index/bp-9fc8445c-f0a6-4958-a07b-ccdf62130530

Tested with:

Mac OS X 10.8.3 Nightly (clean profile): crash
Mac OX X 10.8.3 Aurora (clean profile): crash
Mac OS X 10.8.3 Beta (clean profile): no crash
Mac OS X 10.8.3 Stable (clean profile): no crash
Windows 8 Nightly: crash
Windows 8 Stable: no crash
Thank you for the bug report!

The relevant stack bit is:

#4  0x000000010179ab45 in JSAutoCompartment::JSAutoCompartment (this=0x7fff5fbf0eb8, cx=0x117296400, target=0x0) at jsapi.cpp:1503
#5  0x0000000103a54786 in mozilla::dom::HTMLOptionsCollection::NamedItem (this=0x120410b80, cx=0x117296400, name=@0x7fff5fbf0fc8, error=@0x7fff5fbf0fa0) at HTMLOptionsCollection.cpp:293
#6  0x00000001053274e7 in mozilla::dom::HTMLSelectElement::NamedItem (this=0x1213743e0, aCx=0x117296400, aName=@0x7fff5fbf0fc8, aRv=@0x7fff5fbf0fa0) at HTMLSelectElement.h:212

HTMLOptionsCollection::NamedItem assumes it's called directly from script an hence that the options collection is JS-wrapped, but in this case it's C++ code calling it.

We need to either make sure mOptions is wrapped or we need to do the return-value wrapping ourselves.

Simple testcase:

  data:text/html,<select><option id="foo"><script>alert(document.querySelector("select").namedItem("foo"))</script>
Blocks: 841488
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Severity: normal → critical
Keywords: crash, reproducible
Hardware: x86 → All
Summary: opening railsdiff crashes nightly and aurora → opening railsdiff crashes Firefox in mozilla::dom::HTMLOptionsCollection::NamedItem @ JSAutoCompartment::JSAutoCompartment
Assignee: nobody → Ms2ger
Attached patch Patch v1 (obsolete) — Splinter Review
This is probably the easiest solution.
Attachment #756435 - Flags: review?(bzbarsky)
Attached patch Patch v1.1Splinter Review
In case you prefer a patch that builds.
Attachment #756435 - Attachment is obsolete: true
Attachment #756435 - Flags: review?(bzbarsky)
Attachment #756507 - Flags: review?(bzbarsky)
Comment on attachment 756507 [details] [diff] [review]
Patch v1.1

>+/* static */ HTMLOptionElement*

That comment is wrong.  Please remove it.

>+  HTMLOptionElement? namedItem(DOMString name);

I think we should get the spec adjusted like this as well.

r=me; I like this.  ;)
Attachment #756507 - Flags: review?(bzbarsky) → review+
Oh, and I assume this applies fine to Aurora, right?
https://hg.mozilla.org/mozilla-central/rev/b7344a8ee6f2
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Attached patch Patch for auroraSplinter Review
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 841488
User impact if declined: null pointer dereference crashes
Testing completed (on m-c, etc.): on m-c, passes tests, manually checked original site
Risk to taking this patch (and alternatives if risky): low risk. Alternative is backing out, but that's probably riskier.
String or IDL/UUID changes made by this patch: None.
Attachment #756949 - Flags: approval-mozilla-aurora?
Attachment #756949 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Target Milestone: mozilla24 → mozilla23
Target milestone is when it landed on trunk
Target Milestone: mozilla23 → mozilla24
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.