Closed Bug 1381541 Opened 7 years ago Closed 7 years ago

Crash near null [@GetBoolFlag | mozilla::HTMLEditor::NodeIsBlockStatic]

Categories

(Core :: DOM: Editor, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- wontfix
firefox55 --- wontfix
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: jkratzer, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

(Keywords: crash, csectype-nullptr, testcase)

Crash Data

Attachments

(2 files)

Attached file trigger.html
Testcase found while fuzzing mozilla-central rev 20170717-aff336ac161d.

ASAN:DEADLYSIGNAL
=================================================================
==2983==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000001c (pc 0x7f1fdd5556a7 bp 0x7fff87a70e10 sp 0x7fff87a70d40 T0)
==2983==The signal is caused by a READ memory access.
==2983==Hint: address points to the zero page.
    #0 0x7f1fdd5556a6 in GetBoolFlag /home/worker/workspace/build/src/dom/base/nsINode.h:1592:12
    #1 0x7f1fdd5556a6 in IsElement /home/worker/workspace/build/src/dom/base/nsINode.h:436
    #2 0x7f1fdd5556a6 in IsHTMLElement /home/worker/workspace/build/src/dom/base/nsINode.h:624
    #3 0x7f1fdd5556a6 in IsAnyOfHTMLElements<nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *, nsIAtom *> /home/worker/workspace/build/src/dom/base/nsINode.h:635
    #4 0x7f1fdd5556a6 in mozilla::HTMLEditor::NodeIsBlockStatic(nsINode const*) /home/worker/workspace/build/src/editor/libeditor/HTMLEditor.cpp:761
    #5 0x7f1fdd565ff8 in mozilla::HTMLEditor::GetCSSBackgroundColorState(bool*, nsAString&, bool) /home/worker/workspace/build/src/editor/libeditor/HTMLEditor.cpp:1851:11
    #6 0x7f1fdd63ab4f in nsHighlightColorStateCommand::GetCurrentState(nsIEditor*, nsICommandParams*) /home/worker/workspace/build/src/editor/composer/nsComposerCommands.cpp:831:29
    #7 0x7f1fdd6376cc in nsMultiStateCommand::GetCommandStateParams(char const*, nsICommandParams*, nsISupports*) /home/worker/workspace/build/src/editor/composer/nsComposerCommands.cpp:609:12
    #8 0x7f1fdb764103 in nsControllerCommandTable::GetCommandState(char const*, nsICommandParams*, nsISupports*) /home/worker/workspace/build/src/dom/commandhandler/nsControllerCommandTable.cpp:177:26
    #9 0x7f1fdb75b3e3 in GetCommandStateWithParams /home/worker/workspace/build/src/dom/commandhandler/nsBaseCommandController.cpp:168:25
    #10 0x7f1fdb75b3e3 in non-virtual thunk to nsBaseCommandController::GetCommandStateWithParams(char const*, nsICommandParams*) /home/worker/workspace/build/src/dom/commandhandler/nsBaseCommandController.cpp:156
    #11 0x7f1fdb760bec in nsCommandManager::GetCommandState(char const*, mozIDOMWindowProxy*, nsICommandParams*) /home/worker/workspace/build/src/dom/commandhandler/nsCommandManager.cpp:187:29
    #12 0x7f1fdbc8ecda in nsHTMLDocument::QueryCommandState(nsAString const&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:3527:16
    #13 0x7f1fdb184203 in mozilla::dom::HTMLDocumentBinding::queryCommandState(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:996:21
    #14 0x7f1fdb4aa940 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3060:13
    #15 0x7f1fe1a98434 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:293:15
    #16 0x7f1fe1a98434 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:470
    #17 0x7f1fe1a8121b in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:521:12
    #18 0x7f1fe1a8121b in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3060
    #19 0x7f1fe1a67f48 in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:410:12
    #20 0x7f1fe1a9ad47 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:699:15
Crash Signature: [@ mozilla::HTMLEditor::NodeIsBlockStatic ]
Priority: -- → P1
Assignee: nobody → m_kato
Comment on attachment 8898660 [details]
Bug 1381541 - queryCommandState should consider that parent node of selection is null.

https://reviewboard.mozilla.org/r/170050/#review175204

Hmm, really tricky testcase.
Attachment #8898660 - Flags: review?(masayuki) → review+
This is interesting case. If we keep returning actual range object from Selection.getRangeAt(), nsRange perhaps should reject odd DOM point which cannot be in the Selection which the nsRange object belongs to.
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/fdfce8c8a33a
queryCommandState should consider that parent node of selection is null. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/fdfce8c8a33a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Did you want to nominate this for Beta uplift or let it ride the 57 train?
Flags: needinfo?(m_kato)
Flags: in-testsuite+
Comment on attachment 8898660 [details]
Bug 1381541 - queryCommandState should consider that parent node of selection is null.

Approval Request Comment
[Feature/Bug causing the regression]:
No
[User impact if declined]:
Web content can crash Firefox's content process.

[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]: 
unnecessary

[List of other uplifts needed for the feature/fix]:
Nothing

[Is the change risky?]:
No

[Why is the change risky/not risky?]:
Simply nullptr check when crash situation.

[String changes made/needed]:
No
Flags: needinfo?(m_kato)
Attachment #8898660 - Flags: approval-mozilla-beta?
Comment on attachment 8898660 [details]
Bug 1381541 - queryCommandState should consider that parent node of selection is null.

Fix a crash. Beta56+.
Attachment #8898660 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Makoto Kato [:m_kato] from comment #7)
> [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]: 
> unnecessary

Setting qe-verify- based on Makoto Kato's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.