Closed Bug 1343886 Opened 3 years ago Closed 3 years ago

Crash [@nsTextControlFrame::EnsureEditorInitialized]

Categories

(Core :: Layout: Form Controls, defect, critical)

54 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: jkratzer, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(4 keywords)

Attachments

(2 files)

Attached file Testcase
Testcase found by fuzzing on mozilla-central rev 20170301-34c6c2f302e7.

ASAN:DEADLYSIGNAL
=================================================================
==6075==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000008c (pc 0x7f46ee5e38f5 bp 0x7ffcf04d3670 sp 0x7ffcf04d3400 T0)
    #0 0x7f46ee5e38f4 in nsTextControlFrame::EnsureEditorInitialized() /home/worker/workspace/build/src/layout/forms/nsTextControlFrame.cpp:247:7
    #1 0x7f46ec3b1492 in nsTextEditorState::GetSelectionRange(int*, int*) /home/worker/workspace/build/src/dom/html/nsTextEditorState.cpp:1565:17
    #2 0x7f46ec20fd17 in GetSelectionStart /home/worker/workspace/build/src/dom/html/HTMLInputElement.cpp:6452:17
    #3 0x7f46ec20fd17 in mozilla::dom::HTMLInputElement::GetSelectionStart(mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/html/HTMLInputElement.cpp:6438
    #4 0x7f46eb8f6970 in mozilla::dom::HTMLInputElementBinding::get_selectionStart(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLInputElement*, JSJitGetterCallArgs) /home/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLInputElementBinding.cpp:2884:28
    #5 0x7f46ebaecb06 in mozilla::dom::GenericBindingGetter(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2843:13
    #6 0x7f46f15ec43f in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #7 0x7f46f15ec43f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:448
    #8 0x7f46f15edb09 in InternalCall /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:493:12
    #9 0x7f46f15edb09 in Call /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512
    #10 0x7f46f15edb09 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:627
    #11 0x7f46f24f028a in CallGetter /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1806:16
    #12 0x7f46f24f028a in GetExistingProperty<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:1854
    #13 0x7f46f24f028a in NativeGetPropertyInline<js::AllowGC::CanGC> /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2085
    #14 0x7f46f24f028a in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/NativeObject.cpp:2119
    #15 0x7f46f15f507e in GetProperty /home/worker/workspace/build/src/js/src/vm/NativeObject.h:1435:12
Flags: in-testsuite?
Blocks: domino
No longer blocks: 1172704
Looks like a recent regression, from bug 1342197.  good rev: c52653ed3dd7, bad rev: 17a2b5d9827b
Blocks: 1342197
Has Regression Range: --- → yes
Has STR: --- → yes
Flags: needinfo?(bzbarsky)
Keywords: regression
Version: unspecified → 54 Branch
Simpler testcase:

  data:text/xml,<input xmlns="http://www.w3.org/1999/xhtml"><script>document.documentElement.selectionStart</script></input>

The problem is that I changed to just checking for non-null primary frame instead of checking for nsITextControlFrame, and in the testcase of this bug the <input> is a root element and hence has a frame but nt a text control frame...

The patches in bug 1343037 will fix it by moving the frame check into the editor state, which knows what's going on in this case.  But in the meantime I should probably do a simple bandaid.
Flags: needinfo?(bzbarsky)
Assignee: nobody → bzbarsky
MozReview-Commit-ID: FRzdvTLMAID
Attachment #8843542 - Flags: review?(ehsan)
Attachment #8843542 - Flags: review?(ehsan) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/346de6bc908a
Handle input or textarea elements having a non-textcontrol frame better.  r=ehsan
https://hg.mozilla.org/mozilla-central/rev/346de6bc908a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Aurora approval on this when you get a chance.
Flags: needinfo?(bzbarsky)
Flags: in-testsuite?
Flags: in-testsuite+
Comment on attachment 8843542 [details] [diff] [review]
Handle input or textarea elements having a non-textcontrol frame better

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1342197
[User impact if declined]: Reliable null-deref crashes that are trivial to trigger.
[Is this code covered by automated tests?]:  Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]:  No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No, just adds a null-check that went missing.
[Why is the change risky/not risky?]:
[String changes made/needed]: None
Flags: needinfo?(bzbarsky)
Attachment #8843542 - Flags: approval-mozilla-aurora?
Comment on attachment 8843542 [details] [diff] [review]
Handle input or textarea elements having a non-textcontrol frame better

Fix a crash. Aurora54+.
Attachment #8843542 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.