Issue metadata
Sign in to add a comment
|
Heap-use-after-free in blink::LayoutBlockFlow::determineStartPosition |
||||||||||||||||||||||
Issue descriptionDetailed report: https://cluster-fuzz.appspot.com/testcase?key=6198417501192192 Fuzzer: ifratric-browserfuzzer-v3 Job Type: mac_asan_chrome Platform Id: mac Crash Type: Heap-use-after-free READ 8 Crash Address: 0x60e0000257e0 Crash State: blink::LayoutBlockFlow::determineStartPosition blink::LayoutBlockFlow::layoutRunsAndFloats blink::LayoutBlockFlow::layoutInlineChildren Sanitizer: address (ASAN) Recommended Security Severity: High Regressed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=451977:452017 Reproducer Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96_YhhEf1iAFf7oUKovaQO_hao81ZqmcWDxgJa5W5CVPYc_2KHn50QJfdkoO_h8kPmQfFC2ljyN9HbEA5FuK3djWeKgb2VpmHnj8tIotWUL6x88qSvlnyvuOCZBAeJYENA_UJHY_SJMlgirPYrqN05YyKSsPEag9fqnMfCzQ56O4XeDJGa1MBxyFozuj57Griq__dNkbPZn6hhb8_qji6nFugLIcX5Px6sVGxTLlHwEiwnIaPVX2NXWsJ6O36qJA0z9xeMy4JKmCdE-H_MsGKn46mbFHmBEAHr1ikbsrBcFI6-hPEpEc9wPF5naOrLNQmM361mmLGNwpKlTFPdcRtUbEessOMWGirp14CN0WkTwSuNt-tM?testcase_id=6198417501192192 Issue filed automatically. See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Feb 24 2017
Suspecting https://chromium.googlesource.com/chromium/src/+/eff357ef3a21515a5bfaa487b687d7d7353024f2 which is in the regression range. rune@, could you please take a look?
,
Feb 25 2017
,
Feb 25 2017
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 25 2017
,
Feb 25 2017
,
Feb 26 2017
Reverting https://codereview.chromium.org/2473743003 makes this go away.
,
Feb 26 2017
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 26 2017
Started taking a look.
,
Feb 28 2017
The investigation so far has led to the fact that in LayoutBlockFlow::layoutInlineChildren when we check selfNeedsLayout() it is currently false whereas before my patch it was false. Seems like due to this patch I am not correctly setting the selfNeedsLayout bit. Stumped so as to why that is happening due to my patch. Currently investigating.
,
Feb 28 2017
Have been investigating and logging both with and without my patch. The comment above is incorrect. selfNeedsLayout is false in both cases. In fact the callstack is identical in both cases. Until we hit https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp?q=LayoutBlockFlowLine.cpp&dr&l=2016 Where the isDirty information is incorrect. Sorry about the last statement.
,
Mar 1 2017
Hi, we were able to reduce the test case to the following:
<br>
<dialog id="dialog" open="true" style="position: relative;">
<input id="input" mode="showing" value="test">
<script>
input.select();
document.execCommand("delete");
getComputedStyle(input).color;
dialog.close();
getComputedStyle(document.body).width;
</script>
Putting in console.log statements before and after dialog.close() show that the crash happens in that code.
The call stack looks like:
dialog.close()
HTMLDialogElement::closeDialog
Document::removeFromTopLayer()
Element::setIsInTopLayer()
Node::lazyReattachIfAttached() - a function I have changed. Looking into this now.
If I don't have a fix in by EoD Sydney I will revert the patch leaving plenty of time to Branch point which I believe is March 2
,
Mar 1 2017
Reverted this change. Running clusterfuzz again to verify that the situation has been fixed.
,
Mar 1 2017
Simplified the test a bit:
<!DOCTYPE html>
<br>
<dialog id="dialog" style="position: relative">
<input>
<script>
dialog.show();
document.body.offsetTop; // force layout
dialog.close();
</script>
,
Mar 1 2017
Thank you Rune! Will use this test case to debug and will hopefully be able to make more progress and reland this patch in a better state. :)
,
Mar 1 2017
One difference is that markDirty() is not called in dirtyLinesFromChangedChild() because it early-outs here with the reverted patch because needsAttach() is true for the <dialog> element: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/line/LineBoxList.cpp?type=cs&l=265-269
,
Mar 2 2017
\o/ Yup, needsAttach() post my patch doesn't mean the same thing anymore. Since needsReattachLayoutTree is true which is why we early exit here. I need to revert this back to only checking for getStyleChangeType being NeedsReattachStyleChange. THANK YOU!!!!!! :D Also I have reverted the patch and will attempt to reland after doing an audit of all the calls to needsReattach(). Also thank you to dstockwell and eae@ for their help IRL.
,
Mar 2 2017
ClusterFuzz has detected this issue as fixed in range 453840:453875. Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6198417501192192 Fuzzer: ifratric-browserfuzzer-v3 Job Type: mac_asan_chrome Platform Id: mac Crash Type: Heap-use-after-free READ 8 Crash Address: 0x60e0000257e0 Crash State: blink::LayoutBlockFlow::determineStartPosition blink::LayoutBlockFlow::layoutRunsAndFloats blink::LayoutBlockFlow::layoutInlineChildren Sanitizer: address (ASAN) Recommended Security Severity: High Regressed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=451977:452017 Fixed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=453840:453875 Reproducer Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96_YhhEf1iAFf7oUKovaQO_hao81ZqmcWDxgJa5W5CVPYc_2KHn50QJfdkoO_h8kPmQfFC2ljyN9HbEA5FuK3djWeKgb2VpmHnj8tIotWUL6x88qSvlnyvuOCZBAeJYENA_UJHY_SJMlgirPYrqN05YyKSsPEag9fqnMfCzQ56O4XeDJGa1MBxyFozuj57Griq__dNkbPZn6hhb8_qji6nFugLIcX5Px6sVGxTLlHwEiwnIaPVX2NXWsJ6O36qJA0z9xeMy4JKmCdE-H_MsGKn46mbFHmBEAHr1ikbsrBcFI6-hPEpEc9wPF5naOrLNQmM361mmLGNwpKlTFPdcRtUbEessOMWGirp14CN0WkTwSuNt-tM?testcase_id=6198417501192192 See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Mar 2 2017
Marking this as fixed as per C#18. And moving all further discussion over to https://codereview.chromium.org/2473743003.
,
Mar 2 2017
,
Mar 2 2017
,
Mar 13 2017
,
Jun 8 2017
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by kerrnel@chromium.org
, Feb 24 2017Owner: imch...@chromium.org
Status: Assigned (was: Untriaged)