Closed Bug 1364133 Opened 3 years ago Closed 2 years ago

Crash [@ mozilla::HTMLEditor::SetInlinePropertyOnTextNode]

Categories

(Core :: DOM: Editor, defect, P3, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: jkratzer, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

(Keywords: crash, csectype-nullptr, testcase)

Crash Data

Attachments

(3 files)

Attached file Testcase
Testcase found while fuzzing mozilla-central rev 20170510-d8762cb96742.

=================================================================
==25903==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f47a70b4bf8 bp 0x7ffd75a6df80 sp 0x7ffd75a6de60 T0)
==25903==The signal is caused by a READ memory access.
==25903==Hint: address points to the zero page.
    #0 0x7f47a70b4bf7 in GetAsText /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/Text.h:46:10
    #1 0x7f47a70b4bf7 in mozilla::HTMLEditor::SetInlinePropertyOnTextNode(mozilla::dom::Text&, int, int, nsIAtom&, nsAString const*, nsAString const&) /home/worker/workspace/build/src/editor/libeditor/HTMLStyleEditor.cpp:335
    #2 0x7f47a7075be0 in mozilla::HTMLEditor::SetInlineProperty(nsIAtom*, nsAString const&, nsAString const&) /home/worker/workspace/build/src/editor/libeditor/HTMLStyleEditor.cpp:210:14
    #3 0x7f47a713b142 in nsFontColorStateCommand::SetState(nsIEditor*, nsString&) /home/worker/workspace/build/src/editor/composer/nsComposerCommands.cpp:812:22
    #4 0x7f47a7138081 in nsMultiStateCommand::DoCommandParams(char const*, nsICommandParams*, nsISupports*) /home/worker/workspace/build/src/editor/composer/nsComposerCommands.cpp:595:12
    #5 0x7f47a539de83 in nsControllerCommandTable::DoCommandParams(char const*, nsICommandParams*, nsISupports*) /home/worker/workspace/build/src/dom/commandhandler/nsControllerCommandTable.cpp:162:26
    #6 0x7f47a53956b3 in DoCommandWithParams /home/worker/workspace/build/src/dom/commandhandler/nsBaseCommandController.cpp:152:25
    #7 0x7f47a53956b3 in non-virtual thunk to nsBaseCommandController::DoCommandWithParams(char const*, nsICommandParams*) /home/worker/workspace/build/src/dom/commandhandler/nsBaseCommandController.cpp:140
    #8 0x7f47a539b2bb in nsCommandManager::DoCommand(char const*, nsICommandParams*, mozIDOMWindowProxy*) /home/worker/workspace/build/src/dom/commandhandler/nsCommandManager.cpp:212:29
    #9 0x7f47a58d49e5 in nsHTMLDocument::ExecCommand(nsAString const&, bool, nsAString const&, nsIPrincipal&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:3346:18
    #10 0x7f47a4e1be1c in mozilla::dom::HTMLDocumentBinding::execCommand(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:835:21
    #11 0x7f47a50e4dfe in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:2954:13
    #12 0x7f47aa8f53c3 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:293:15
    #13 0x7f47aa8f53c3 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:470
    #14 0x7f47aa8ddf6f in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:521:12
    #15 0x7f47aa8ddf6f in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3025
Flags: in-testsuite?
Crash Signature: [@ nsINode::GetAsText ]
Priority: -- → P1
Blocks: 1147412
Priority: P1 → P3
Crash Signature: [@ nsINode::GetAsText ] → [@ nsINode::GetAsText ] [@ mozilla::HTMLEditor::SetInlinePropertyOnTextNode ]
Has Regression Range: --- → yes
Summary: Crash @ [mozilla::HTMLEditor::SetInlinePropertyOnTextNode] → Crash [@ mozilla::HTMLEditor::SetInlinePropertyOnTextNode]
Assignee: nobody → m_kato
Comment on attachment 8917315 [details]
Bug 1364133 - Part 1. Check whether SplitNode returns error.

https://reviewboard.mozilla.org/r/188304/#review193562

::: editor/libeditor/HTMLStyleEditor.cpp:293
(Diff revision 1)
> -    text = SplitNode(aText, aEndOffset, rv)->GetAsText();
> +    text = SplitNode(*text, aEndOffset, rv);
>      NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
>    }

However, you're patch doesn't check if the result is nullptr nor a Text. If it's not necessary, r+, otherwise, please fix add the check
Attachment #8917315 - Flags: review?(masayuki) → review+
> However, you're patch doesn't check if the result is nullptr nor a Text. If it's not necessary, r+, otherwise, please fix add the check

unnecessary.
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/92d2ebea6fbc
Part 1. Check whether SplitNode returns error. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/095f6f26bbce
Part 2. Add test. r=masayuki
You need to log in before you can comment on or make changes to this bug.