webkit-text-stroke-color makes insertText to crash |
||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=4565231459368960 Fuzzer: ochang_domfuzzer Job Type: linux_asan_chrome_mp Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000040 Crash State: blink::IsTabHTMLSpanElement blink::EditingStyle::Init blink::EditingStyle::Create Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=506675:506834 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4565231459368960 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Oct 27 2017
Using the provided regression range and GIT Blame results assigning to the possible suspect as per the change made for the file, “EditingUtilities.cpp” Suspecting Commit# https://chromium.googlesource.com/chromium/src/+/52d55d36c78e4772e0e3499409dc6f6dcabde00e @yosin -- Could you please look into this issue, kindly reassign if it has nothing to do with your changes. Thanks.
,
Oct 27 2017
,
Oct 30 2017
Lower to Pri-3, since this is caused by unusual HTML.
"insertHTML" command moves STYLE after BODY and doesn't insert "x".
I'm not sure why "insertHTML" does so.
HTML after insertHTML
<html><head></head><body><script>
document.designMode = 'on';
document.execCommand('selectAll');
document.execCommand('insertHTML', 'x');
document.execCommand('selectAll');
//document.execCommand('InsertText', false, '\t');
</script>
</body><style>
head { -webkit-text-stroke-color: black; display: list-item; }
</style></html>
# Minimized HTML
<!doctype html>
<style>
head { -webkit-text-stroke-color: black; display: list-item; }
</style>a<script>
document.designMode = 'on';
document.execCommand('selectAll');
document.execCommand('insertHTML', 'x');
document.execCommand('selectAll');
document.execCommand('insertText', false, '\t');
</script>
,
Nov 1 2017
,
Nov 15 2017
I have no idea why GetComputedStyle() returns null. But do we really care about it? Guess we just add a nullptr check to GetComputedStyle(), since an unrendered element can't be a tab span.
,
Nov 16 2017
Is |node| parameter of IsTabHTMLSpanElement() connected?
Note: |node| parameter comes from |end_position| of below:
InsertTextCommand::DoApply()
...
if (text_ == "\t" && IsRichlyEditablePosition(start_position)) {
// Set by here
end_position = InsertTab(start_position, editing_state);
if (editing_state->IsAborted())
return;
start_position =
PreviousPositionOf(end_position, PositionMoveType::kGraphemeCluster);
if (placeholder.IsNotNull())
RemovePlaceholderAt(placeholder);
// Does |end_position| alive after |RemovePlaceholderAt()|?
...
// Is |end_position| really valid?
SetEndingSelectionWithoutValidation(start_position, end_position);
// Handle the case where there is a typing style.
if (EditingStyle* typing_style =
GetDocument().GetFrame()->GetEditor().TypingStyle()) {
// Use by here
typing_style->PrepareToApplyAt(end_position,
EditingStyle::kPreserveWritingDirection);
,
Nov 16 2017
|node| is connected. The root cause seems to be that IsTabHTMLSpanElement() is called before StyleClean -- at least that's what happened to this bug.
,
Nov 16 2017
Thanks for explanation! I think the patch[1] did wrong thing. It seems the author though |Node::ComputedStyle()| can return computed style anytime[2] but not. The original intention is checking class=Apple-Tab-Span and we changed it to style=white-space:pre. Thus, I think we should check style=white-space:pre attribute instead of computed style. [1] http://crrev.com/c/697044: Make InsertText command not to split SPAN element containing TAB character [2] CSSStyleDeclaration* LocalDOMWindow::getComputedStyle( Element* elt, const String& pseudo_elt) const { DCHECK(elt); return CSSComputedStyleDeclaration::Create(elt, false, pseudo_elt); }
,
Nov 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9bbd081d6401276e8173cf0e33b9f93cba1c1b01 commit 9bbd081d6401276e8173cf0e33b9f93cba1c1b01 Author: Xiaocheng Hu <xiaochengh@chromium.org> Date: Mon Nov 20 07:32:19 2017 Ensure clean style when checking style in IsTabHTMLSpanElement IsTabSpanElement() checks the 'white-space' style of an element, but may be called with dirty style, and hence causing crash. This patch adds a style update in the function to stop the crash. Bug: 778901 , 779527 Change-Id: I0c0d22f1a94ce06abd5fd146421cf00e4dbf6ac1 Reviewed-on: https://chromium-review.googlesource.com/772832 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#517774} [modify] https://crrev.com/9bbd081d6401276e8173cf0e33b9f93cba1c1b01/third_party/WebKit/Source/core/editing/EditingUtilities.cpp [modify] https://crrev.com/9bbd081d6401276e8173cf0e33b9f93cba1c1b01/third_party/WebKit/Source/core/editing/commands/InsertTextCommandTest.cpp
,
Nov 21 2017
ClusterFuzz has detected this issue as fixed in range 517712:517848. Detailed report: https://clusterfuzz.com/testcase?key=4565231459368960 Fuzzer: ochang_domfuzzer Job Type: linux_asan_chrome_mp Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000040 Crash State: blink::IsTabHTMLSpanElement blink::EditingStyle::Init blink::EditingStyle::Create Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=506675:506834 Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=517712:517848 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4565231459368960 See https://github.com/google/clusterfuzz-tools for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Nov 21 2017
ClusterFuzz testcase 4565231459368960 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Nov 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e8fe460ce4cb3d8908d96b4c157ef91119ad3ff2 commit e8fe460ce4cb3d8908d96b4c157ef91119ad3ff2 Author: Xiaocheng Hu <xiaochengh@chromium.org> Date: Tue Nov 28 21:08:19 2017 Ensure clean style when checking style in IsTabHTMLSpanElement IsTabSpanElement() checks the 'white-space' style of an element, but may be called with dirty style, and hence causing crash. This patch adds a style update in the function to stop the crash. TBR=xiaochengh@chromium.org (cherry picked from commit 9bbd081d6401276e8173cf0e33b9f93cba1c1b01) Bug: 778901 , 779527 Change-Id: I0c0d22f1a94ce06abd5fd146421cf00e4dbf6ac1 Reviewed-on: https://chromium-review.googlesource.com/772832 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#517774} Reviewed-on: https://chromium-review.googlesource.com/794631 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#591} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/e8fe460ce4cb3d8908d96b4c157ef91119ad3ff2/third_party/WebKit/Source/core/editing/EditingUtilities.cpp |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by ClusterFuzz
, Oct 27 2017