New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 670927 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
NOT IN USE
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in void blink::PODIntervalTree<blink::LayoutUnit, blink::FloatingObject*>::searchFo

Project Member Reported by ClusterFuzz, Dec 3 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4646216106508288

Fuzzer: bj_broddelwerk
Job Type: linux_asan_chrome_v8_arm
Platform Id: linux

Crash Type: Heap-use-after-free READ 1
Crash Address: 0xb0151a38
Crash State:
  void blink::PODIntervalTree<blink::LayoutUnit, blink::FloatingObject*>::searchFo
  blink::FloatingObjects::logicalLeftOffsetForPositioningFloat
  blink::LayoutBlockFlow::computeLogicalLocationForFloat
  
Recommended Security Severity: High

Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_v8_arm&range=434929:434986

Minimized Testcase (6.92 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96pWQLOFwIX_wv4gFZ0Tr9n2wxgx3B8yAWw2t_3Ib77BltEOv6X1gf03SPd6j2CIZ4gDNGCL8JLkW8YUKkzwb913sRQDC5WQM1KtkDKATuSo-xDAOcZlJn9yVX3qNslwqTcgfOI0ztbRVAfO_cBgZNOUdoxxA?testcase_id=4646216106508288

Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Dec 3 2016

Labels: M-56
Project Member

Comment 2 by sheriffbot@chromium.org, Dec 3 2016

Labels: ReleaseBlock-Beta
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
Project Member

Comment 3 by sheriffbot@chromium.org, Dec 3 2016

Labels: Pri-1
Components: Blink>Layout
Owner: msten...@opera.com
Status: Assigned (was: Untriaged)
mstensho@, it seems you have been working on blink layout lately. Could you take a look to see if any of your CL caused this problem?
Project Member

Comment 5 by sheriffbot@chromium.org, Dec 4 2016

Labels: -Security_Impact-Head Security_Impact-Beta

Comment 7 by msten...@opera.com, Dec 5 2016

tc.html
534 bytes View Download

Comment 8 by msten...@opera.com, Dec 6 2016

Testcase without contentEditable. It's really -webkit-line-break:after-white-space (which is automatically turned on for every editable element) that triggers a special code path in line layout (BreakingContext::skipTrailingWhitespace()).
tc2.html
457 bytes View Download

Comment 9 by msten...@opera.com, Dec 6 2016

Cc: e...@chromium.org szager@chromium.org
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f08e63f7cbb8de6ad0b14c0174a75386a4a2863

commit 2f08e63f7cbb8de6ad0b14c0174a75386a4a2863
Author: mstensho <mstensho@opera.com>
Date: Wed Dec 07 00:28:41 2016

Never position a float after it has been placed.

When a float is marked as "placed" (which happens in
LayoutBlockFlow::placeNewFloats()), it means that it has been added to a float
interval tree. It is not allowed to move a float afterwards (unless we remove
and re-insert the floats somehow, e.g. by re-laying out its containing block).
Otherwise, the interval tree may get out of sync with reality, and we may fail
to find the reference to a FloatingObject in the interval tree when deleting a
FloatingObject, so that we end up deleting the FloatingObject, but not the
reference to it in the interval tree (which will remain there, pointing to a
now dead object).

This could happen when LayoutBlockFlow::removeFloatingObjectsBelow() was called
during pagination. We sometimes need to re-lay out a line because the line or
floats next to the line get pushed to the next fragmentainer. As part of that,
we also need to get rid of the floats that we thought would sit beside the
line, and re-position them.

BUG= 670927 

Review-Url: https://codereview.chromium.org/2553923003
Cr-Commit-Position: refs/heads/master@{#436776}

[add] https://crrev.com/2f08e63f7cbb8de6ad0b14c0174a75386a4a2863/third_party/WebKit/LayoutTests/fast/block/float/line-break-after-white-space-crash.html
[modify] https://crrev.com/2f08e63f7cbb8de6ad0b14c0174a75386a4a2863/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
[modify] https://crrev.com/2f08e63f7cbb8de6ad0b14c0174a75386a4a2863/third_party/WebKit/Source/core/layout/line/BreakingContextInlineHeaders.h

Status: Fixed (was: Assigned)
Project Member

Comment 12 by ClusterFuzz, Dec 7 2016

ClusterFuzz has detected this issue as fixed in range 436692:436783.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4646216106508288

Fuzzer: bj_broddelwerk
Job Type: linux_asan_chrome_v8_arm
Platform Id: linux

Crash Type: Heap-use-after-free READ 1
Crash Address: 0xb0151a38
Crash State:
  void blink::PODIntervalTree<blink::LayoutUnit, blink::FloatingObject*>::searchFo
  blink::FloatingObjects::logicalLeftOffsetForPositioningFloat
  blink::LayoutBlockFlow::computeLogicalLocationForFloat
  
Recommended Security Severity: High

Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_v8_arm&range=434929:434986
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_v8_arm&range=436692:436783

Minimized Testcase (6.92 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96pWQLOFwIX_wv4gFZ0Tr9n2wxgx3B8yAWw2t_3Ib77BltEOv6X1gf03SPd6j2CIZ4gDNGCL8JLkW8YUKkzwb913sRQDC5WQM1KtkDKATuSo-xDAOcZlJn9yVX3qNslwqTcgfOI0ztbRVAfO_cBgZNOUdoxxA?testcase_id=4646216106508288

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.
Project Member

Comment 13 by sheriffbot@chromium.org, Dec 7 2016

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 14 by sheriffbot@chromium.org, Dec 9 2016

Labels: Merge-Request-56

Comment 15 by dimu@chromium.org, Dec 9 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
https://chromium.googlesource.com/chromium/src/+/596aa530f12f7777ec6336c891930a5c6770fad4 (what introduced this bug) landed on the 29th of November. M56 was branched on the 18th of November. So, unless I'm horribly mistaken, M56 doesn't have this bug.
Project Member

Comment 17 by sheriffbot@chromium.org, Dec 12 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 18 by msten...@opera.com, Dec 12 2016

Labels: -Merge-Approved-56
Labels: -ReleaseBlock-Beta -M-56 M-57
Project Member

Comment 20 by sheriffbot@chromium.org, Mar 15 2017

Labels: -Restrict-View-SecurityNotify allpublic
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