FormatBlock command crashes with unusual HTML |
||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5369690030080000 Fuzzer: ochang_domfuzzer Job Type: linux_tsan_chrome_mp Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000010 Crash State: blink::Node::IsDescendantOf blink::SelectionForParagraphIteration blink::ApplyBlockElementCommand::DoApply Sanitizer: thread (TSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=575974:575975 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5369690030080000 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Aug 10
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/7bd29404a6ab8d36bdff4123ae522fcd9068344b ([Blink] Avoid crossing editing boundaries selection.). If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
,
Aug 10
,
Aug 10
Xiaocheng, could you please take a look, since this is related to |-webkit-user-modify|. Probably another issue exposed by my change.
Simplified a little of the test case:
<head>
<style>
*{-webkit-user-modify:read-write;}
.CLASS4{-webkit-user-modify:read-only;}
</style>
<script>
function fuzz() {
document.execCommand("selectAll");
document.execCommand("FormatBlock",false,"section");
}
setTimeout(fuzz);
</script>
<table></table>
<kbd class="CLASS4 CLASS14">
<button>
There are several issues I saw from this crash:
1) ClusterFuzz is using -webkit-user-modify property, which is a non-standard, we are not encouraged to use this I guess.
https://www.chromestatus.com/feature/4725222129270784
2) <kbr> element is not editable due to |-webkit-user-modify: read-only;|, but it's child element <button> is still editable because of |-webkit-user-modify: read-write;|, I think it should inherit <kbr>'s style.
3) After editing boundary adjustment, the selection is:
BODY (editable)
B TABLE (editable)
#text "\n"
#text "\n"
#comment (editable)
#text "\n"
E KBD class="CLASS4"
#text "\n"
BUTTON (editable)
#text "\n"
base: offsetInAnchor[0]
extent: beforeAnchor
But after |SelectionAdjuster::AdjustSelectionType()|, selection is
BODY (editable)
S TABLE (editable)
#text "\n"
#text "\n"
#comment (editable)
#text "\n"
E KBD class="CLASS4"
#text "\n"
BUTTON (editable)
#text "\n"
start: offsetInAnchor[0]
end: offsetInAnchor[0]
Note that extent is changed from before <kbd> to inside <kbd>, which is clearly not correct.
This caused in |blink::SelectionForParagraphIteration()|, |new_selection.VisibleEnd()| becomes null position, so it crashes in the end.
There is another type of crash with this test case from spell checker code path.. But that's probably another issue.
,
Aug 10
,
Aug 13
Lower to Pri-3 because real world usage of "FormatBlock" command is low and this crash is caused by unusual HTML.
Hit DCHECK(!end_of_selection.IsNull()) in ApplyBlockElementCommand::DoApply().
Where selection = base:<body>@0 extent:B@0
VisiblePosition end_of_selection = selection.VisibleEnd();
Minimal HTML: Using document.designMode='on' instead of <style>*...</style> doesn't reproduce.
<style>*{-webkit-user-modify:read-write;}</style>
<b style="-webkit-user-modify:read-only;"><button></button></b>
<script>
document.execCommand('selectAll');
document.execCommand('FormatBlock', false, 'section');
</script>
,
Aug 13
,
Aug 13
As analyzed in #4, the root cause might be in SelectionAdjuster::AdjustSelectionType. However, fixing it might be too risky. As this crash requires an unusual usage of -webkit-user-modify, let's supress it in the editing command for now with ABORT_EDITING_COMMAND_IF.
,
Aug 14
,
Aug 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e27bd822258532a2bb89734ca8cebadebaf411c5 commit e27bd822258532a2bb89734ca8cebadebaf411c5 Author: Xiaocheng Hu <xiaochengh@chromium.org> Date: Tue Aug 14 04:51:14 2018 Abort formatBlock command when paragraph range calculation errs FormatBlock command requires valid paragraph range to operate on, but fails to obtain it in some edge scenarios. This patch makes the command abort so that it doesn't crash the renderer. Bug: 873084 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Iccfc83be7bc2db950e9fad00974ab6a5e67f9548 Reviewed-on: https://chromium-review.googlesource.com/1173432 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#582833} [modify] https://crrev.com/e27bd822258532a2bb89734ca8cebadebaf411c5/third_party/blink/renderer/core/editing/commands/apply_block_element_command.cc [modify] https://crrev.com/e27bd822258532a2bb89734ca8cebadebaf411c5/third_party/blink/renderer/core/editing/commands/apply_block_element_command_test.cc
,
Aug 14
Now the repro in #6 no longer crashes, but the original repro still does...
,
Aug 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eaa72521e4677a60a486ff604e94a55a77a9a229 commit eaa72521e4677a60a486ff604e94a55a77a9a229 Author: Xiaocheng Hu <xiaochengh@chromium.org> Date: Wed Aug 15 02:06:50 2018 Abort formatBlock command when SelectionForParagraphIteration() errs The editing command requires SelectionForParagraphIteration() to figure out the correct paragraph range, which however may fail due to invalid input. This patch makes the command abort so that it doesn't crash the renderer. Bug: 873084 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I86c77903e218ff4b564bce9122315b99896b01ed Reviewed-on: https://chromium-review.googlesource.com/1175244 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#583128} [modify] https://crrev.com/eaa72521e4677a60a486ff604e94a55a77a9a229/third_party/blink/renderer/core/editing/commands/apply_block_element_command.cc [modify] https://crrev.com/eaa72521e4677a60a486ff604e94a55a77a9a229/third_party/blink/renderer/core/editing/commands/apply_block_element_command_test.cc [modify] https://crrev.com/eaa72521e4677a60a486ff604e94a55a77a9a229/third_party/blink/renderer/core/editing/commands/editing_commands_utilities.cc
,
Aug 15
ClusterFuzz has detected this issue as fixed in range 583126:583129. Detailed report: https://clusterfuzz.com/testcase?key=5369690030080000 Fuzzer: ochang_domfuzzer Job Type: linux_tsan_chrome_mp Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000010 Crash State: blink::Node::IsDescendantOf blink::SelectionForParagraphIteration blink::ApplyBlockElementCommand::DoApply Sanitizer: thread (TSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=575974:575975 Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=583126:583129 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5369690030080000 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.
,
Aug 15
ClusterFuzz testcase 5369690030080000 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by ClusterFuzz
, Aug 10Labels: Test-Predator-Auto-Components