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

RESOLVED FIXED in Firefox 56

Status

()

Core
Editor
P1
normal
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: jkratzer, Assigned: m_kato)

Tracking

(Blocks: 1 bug, {crash, csectype-nullptr, testcase})

unspecified
mozilla57
crash, csectype-nullptr, testcase
Points:
---
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox55 wontfix, firefox56 fixed, firefox57 fixed)

Details

(crash signature)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

5 months ago
Created attachment 8887111 [details]
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 hidden (mozreview-request)

Comment 2

4 months ago
mozreview-review
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.

Comment 4

4 months ago
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

Comment 5

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fdfce8c8a33a
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Did you want to nominate this for Beta uplift or let it ride the 57 train?
status-firefox55: --- → wontfix
status-firefox56: --- → affected
status-firefox-esr52: --- → wontfix
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 8

4 months ago
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+

Comment 9

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/4572266eacb3
status-firefox56: affected → fixed
(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.