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

Issue 713671 link

Starred by 1 user

Issue metadata

Status: Verified
Owner: ----
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , All , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Invalid-address in blink::LayoutListItem::UpdateListMarkerNumbers

Project Member Reported by ClusterFuzz, Apr 20 2017

Issue description

Project Member

Comment 1 by sheriffbot@chromium.org, Apr 20 2017

Labels: M-59
Project Member

Comment 2 by sheriffbot@chromium.org, Apr 20 2017

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, Apr 20 2017

Labels: Pri-1
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 21 2017

Labels: -Security_Impact-Head Security_Impact-Beta
Project Member

Comment 5 by ClusterFuzz, Apr 21 2017

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=6563941405949952
Project Member

Comment 6 by ClusterFuzz, Apr 22 2017

Detailed report: https://clusterfuzz.com/testcase?key=6563941405949952

Job Type: windows_syzyasan_chrome
Crash Type: Invalid-address READ 4
Crash Address: 0x0000000B
Crash State:
  (unknown)
  
Memory Tool: SYZYASAN

Regressed: https://clusterfuzz.com/revisions?job=windows_syzyasan_chrome&range=465765:465806

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6563941405949952


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
Labels: -M-59 M-60

Comment 8 by mea...@chromium.org, Apr 25 2017

Owner: kouhei@chromium.org
Status: Assigned (was: Untriaged)
The stack traces aren't helpful. From the regression range, https://chromium.googlesource.com/chromium/src/+/b985210b2afe0d77382b6cf818aaee56968b0e6f seems relevant. 

kouhei: Can you please see if this is caused by b985210b2afe0d77382b6cf818aaee56968b0e6f? 

Comment 9 by kouhei@chromium.org, Apr 25 2017

Cc: mea...@chromium.org
Owner: ----
Status: Untriaged (was: Assigned)
I don't think so. This codepath is currently under a flag and never enabled in the attached html.
Thanks for the quick response (and sorry I missed the ES6 modules tag). 
Cc: tasak@chromium.org schenney@chromium.org pdr@chromium.org
Components: Blink>Layout
Labels: OS-Android OS-Chrome OS-Linux OS-Mac
Owner: esprehn@chromium.org
Status: Assigned (was: Untriaged)
Summary: Invalid-address in blink::LayoutListItem::UpdateListMarkerNumbers (was: Invalid-address in (unknown))
I managed to get a stack trace with ASan on Linux:

Received signal 11 SEGV_MAPERR 000000000010
#0 0x5594add75791 __interceptor_backtrace
#1 0x7f1bda2e88fc base::debug::StackTrace::StackTrace()
#2 0x7f1bda2e78fc base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x7f1bbc1a2330 <unknown>
#4 0x7f1bc5d60838 blink::LayoutListItem::UpdateListMarkerNumbers()
#5 0x7f1bc5dcc6cd blink::LayoutObjectChildList::RemoveChildNode()
#6 0x7f1bc5bd2713 blink::LayoutBlockFlow::RemoveChild()
#7 0x7f1bc5dbe7f5 blink::LayoutObject::WillBeDestroyed()
#8 0x7f1bc5c7bb00 blink::LayoutBoxModelObject::WillBeDestroyed()
#9 0x7f1bc5d60191 blink::LayoutListItem::WillBeDestroyed()
#10 0x7f1bc5dc25a7 blink::LayoutObject::Destroy()
#11 0x7f1bc5dc239f blink::LayoutObject::DestroyAndCleanupAnonymousWrappers()
#12 0x7f1bc4cbe109 blink::Node::DetachLayoutTree()
#13 0x7f1bc4ac6792 blink::ContainerNode::DetachLayoutTree()
#14 0x7f1bc4bf61a7 blink::Element::DetachLayoutTree()
#15 0x7f1bc4e1ab01 blink::ElementShadow::AddShadowRoot()
#16 0x7f1bc4bfdaeb blink::Element::CreateShadowRootInternal()
#17 0x7f1bc4bfe63c blink::Element::attachShadow()
#18 0x7f1bc6b230af blink::V8Element::attachShadowMethodCallback()
#19 0x7f1bc8891c6e v8::internal::FunctionCallbackArguments::Call()
#20 0x7f1bc8ad6551 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#21 0x7f1bc8ad39ad v8::internal::Builtin_Impl_HandleApiCall()
#22 0x7f1b883046fd <unknown>
  r8: 00007f1baede0000  r9: 00007f1baeddf5a1 r10: 0000000000000800 r11: 00007f1bbbd3e001
 r12: 0000000000000000 r13: 0000000000000000 r14: 00006120000091c0 r15: 00000fe3f5e66700
  di: 0000000000000010  si: 00000000000000f5  bp: 00007ffebfbfce30  bx: 00007ffebfbfcd80
  dx: 00000fe3f5e4f800  ax: 00000000000c0000  cx: 00000fe3f5e4f800  sp: 00007ffebfbfcd80
  ip: 00007f1bc5d60838 efl: 0000000000010246 cgf: 0000000000000033 erf: 0000000000000004
 trp: 000000000000000e msk: 0000000000000000 cr2: 0000000000000010
[end of stack trace]
Calling _exit(1). Core file will not be generated.
Cc: e...@chromium.org esprehn@chromium.org
Owner: ----
Status: Untriaged (was: Assigned)
This looks like an access into a nullptr inside LayoutListItem::UpdateListMarkerNumbers?

I wonder if this DCHECK would hit it:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutListItem.cpp?q=LayoutListItem::UpdateListMarkerNumbers+package:%5Echromium$&l=526
Status: Available (was: Untriaged)
If a Layout OWNER could pick this up, that'd be great. Thanks.

Comment 14 by e...@chromium.org, Apr 26 2017

Cc: binji@chromium.org
Status: Untriaged (was: Available)
Please leave issues as Untriaged when changing the component. Setting them as to Assigned or Available pretty much guarantees that it'll take much longer for someone to look at them.

The only change in the regression range that looks even remotely related is r465787 (https://codereview.chromium.org/2815793002).

Requested another bisect run as the clusterfuzz regression reporting as been really flaky lately.
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 26 2017

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

commit ea39d1dc3b628c867bb78baddbf44e2f017e0456
Author: eae <eae@chromium.org>
Date: Wed Apr 26 05:34:21 2017

Change DCHECK to CHECK in LayoutListItem::UpdateListMarkerNumbers

Speculative bug fix.

BUG= 713671 
R=kojii@chromium.org

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

[modify] https://crrev.com/ea39d1dc3b628c867bb78baddbf44e2f017e0456/third_party/WebKit/Source/core/layout/LayoutListItem.cpp

Comment 16 by e...@chromium.org, Apr 26 2017

Cc: wangxianzhu@chromium.org
 Issue 713298  has been merged into this issue.
#21: The issues stay on our security sheriff dashboard until they're fixed, so we're keeping our eyes on them, at least. :) For us (security), Untriaged means not assigned Security_Severity, Components, and Security_Impact.
Project Member

Comment 18 by ClusterFuzz, Apr 27 2017

ClusterFuzz has detected this issue as fixed in range 467232:467246.

Detailed report: https://clusterfuzz.com/testcase?key=5008077154942976

Fuzzer: ifratric-browserfuzzer-v3
Job Type: windows_syzyasan_chrome
Platform Id: windows

Crash Type: Invalid-address READ 4
Crash Address: 0x0000000B
Crash State:
  (unknown)
  
Memory Tool: SYZYASAN

Regressed: https://clusterfuzz.com/revisions?job=windows_syzyasan_chrome&range=465765:465806
Fixed: https://clusterfuzz.com/revisions?job=windows_syzyasan_chrome&range=467232:467246

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5008077154942976


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.
Hmm how did the DCHECK -> CHECK fix this, wouldn't it just start crashing now instead of doing an invalid read?
Project Member

Comment 20 by ClusterFuzz, Apr 27 2017

ClusterFuzz has detected this issue as fixed in range 467232:467246.

Detailed report: https://clusterfuzz.com/testcase?key=5008077154942976

Fuzzer: ifratric-browserfuzzer-v3
Job Type: windows_syzyasan_chrome
Platform Id: windows

Crash Type: Invalid-address READ 4
Crash Address: 0x0000000B
Crash State:
  (unknown)
  
Memory Tool: SYZYASAN

Regressed: https://clusterfuzz.com/revisions?job=windows_syzyasan_chrome&range=465765:465806
Fixed: https://clusterfuzz.com/revisions?job=windows_syzyasan_chrome&range=467232:467246

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5008077154942976


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 21 by ClusterFuzz, Apr 27 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Untriaged)
ClusterFuzz testcase 6009982060593152 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 22 by sheriffbot@chromium.org, Apr 27 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -ReleaseBlock-Beta OS-All
Project Member

Comment 24 by sheriffbot@chromium.org, Jun 9 2017

Labels: Merge-Request-60
Project Member

Comment 25 by sheriffbot@chromium.org, Jun 9 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

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

Comment 26 by sheriffbot@chromium.org, Jun 12 2017

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

Comment 27 by sheriffbot@chromium.org, Jun 15 2017

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
Cc: awhalley@chromium.org
Does anything need to be merged here?
Labels: -Merge-Approved-60
Nope, #15 made 60, and clusterfuzz confirmed in #20 it's fixed there.
Project Member

Comment 30 by ClusterFuzz, Jul 14 2017

Labels: Needs-Feedback
ClusterFuzz testcase 6563941405949952 is still reproducing on tip-of-tree build (trunk).

Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.
Labels: -Type-Bug-Security -Security_Severity-High -Security_Impact-Beta Type-Bug
removing from security queue.
Project Member

Comment 32 by sheriffbot@chromium.org, Aug 3 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