New issue
Advanced search Search tips

Issue 799668 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in chrome::GetURLAndTitleToBookmark

Project Member Reported by ClusterFuzz, Jan 6 2018

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  chrome::GetURLAndTitleToBookmark
  chrome::BookmarkCurrentPageIgnoringExtensionOverrides
  StarView::ExecuteCommand
  
Sanitizer: thread (TSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=518240:518474

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

Additional requirements: Requires Gestures

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Jan 6 2018

Components: UI>Browser>Omnibox
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Components: UI>Browser>TabStrip
Owner: fdoray@chromium.org
Status: Assigned (was: Untriaged)
Heh, what a fun key sequence in the testcase.

Roughly it looks like it's possible to hit the bookmark current page shortcut after hitting control-w to close the tab.  The webcontents had already been destroyed, but the focus hadn't yet been removed from the star/bookmark area, so the bookmark commands try to operate on an null webcontents.

At a quick glance at the blamelist,
https://chromium.googlesource.com/chromium/src/+/414d24f7f85dd165674e4ab0c861b7d32971495f
seems relevant.  Assigning appropriately.

fdoray@, can you please take a look?
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 15 2018

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

commit ce5f0539584ec54f21f1d7ea0cb6e931a5b1f9a8
Author: Francois Doray <fdoray@chromium.org>
Date: Mon Jan 15 20:11:23 2018

No-op when there is no active tab in BookmarkCurrentPageIgnoringExtensionOverrides().

When a tab is closed, its WebContents is destroyed. If it's the last
tab in the browser window, an asynchronous
views::NativeWidgetAura::CloseNow() task is posted to close the browser
window. If BookmarkCurrentPageIgnoringExtensionOverrides() is
called before that task runs, it gets a nullptr from
browser->tab_strip_model()->GetActiveWebContents() and tries to
access it, causing a crash.

This CL fixes that issue by checking if the return value of
GetActiveWebContents() is nullptr in
BookmarkCurrentPageIgnoringExtensionOverrides().

Note: This is a bug found by Clusterfuzz. It is extremely hard to
reproduce, so I haven't been able to find the CL that introduced it,
or determine whether if it has always existed.

Bug:  799668 
Change-Id: I11c0639a1e1248b4dd478737bf64ab4119d1f6e6
Reviewed-on: https://chromium-review.googlesource.com/860721
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529324}
[modify] https://crrev.com/ce5f0539584ec54f21f1d7ea0cb6e931a5b1f9a8/chrome/browser/ui/browser_commands.cc
[add] https://crrev.com/ce5f0539584ec54f21f1d7ea0cb6e931a5b1f9a8/chrome/browser/ui/browser_commands_browsertest.cc
[modify] https://crrev.com/ce5f0539584ec54f21f1d7ea0cb6e931a5b1f9a8/chrome/test/BUILD.gn

Project Member

Comment 4 by ClusterFuzz, Jan 16 2018

ClusterFuzz has detected this issue as fixed in range 529323:529324.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  chrome::GetURLAndTitleToBookmark
  chrome::BookmarkCurrentPageIgnoringExtensionOverrides
  StarView::ExecuteCommand
  
Sanitizer: thread (TSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=518240:518474
Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=529323:529324

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

Additional requirements: Requires Gestures

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

Comment 5 by ClusterFuzz, Jan 16 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5973823538331648 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