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

Issue 751401 link

Starred by 1 user

Issue metadata

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

Blocked on:
issue 752596



Sign in to add a comment

Null-dereference READ in guest_view::GuestViewManager::CanEmbedderAccessInstanceID

Project Member Reported by ClusterFuzz, Aug 2 2017

Issue description

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

Fuzzer: ipc_fuzzer_gen
Job Type: linux_asan_chrome_ipc
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000070
Crash State:
  guest_view::GuestViewManager::CanEmbedderAccessInstanceID
  guest_view::GuestViewManager::GetGuestByInstanceIDSafely
  guest_view::GuestViewMessageFilter::OnAttachToEmbedderFrame
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_ipc&range=491035:491089

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


Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: msrchandra@chromium.org
Labels: M-62 Test-Predator-Wrong
Owner: thestig@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL did not provide any possible suspects.
Using Code Search for the file, "guest_view_manager.cc" assigning to concern owner.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/09b9c51e845829bab88d19494467e886520af4e6

@thestig
Predator and CL did not provide any possible suspects.
Using Code Search for the file, "guest_view_manager.cc" assigning to concern owner.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/09b9c51e845829bab88d19494467e886520af4e6

@thestig -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
Cc: fsam...@chromium.org
Owner: ----
Status: Untriaged (was: Assigned)
Nope, not my CL. We may be blowing through the DCHECK() inside GuestViewMessageFilter::OnAttachToEmbedderFrame()?
Components: Platform>Apps>BrowserTag

Comment 5 by lfg@chromium.org, Aug 2 2017

Cc: wjmaclean@chromium.org
Owner: mcnee@chromium.org
Assigning to Kevin for triage, since James is OOO.

Comment 6 by mcnee@chromium.org, Aug 3 2017

The read in GuestViewManager::CanEmbedderAccessInstanceID is of |current_instance_id_|, so yeah, we have a null GuestViewManager.

I notice that GuestViewMessageFilter::OnAttachGuest handles such a case by returning instead of DCHECKing, so we could do that, or as mentioned in  crbug.com/751376 , maybe we just want to kill the renderer?

Comment 7 by mcnee@chromium.org, Aug 3 2017

Cc: kenrb@chromium.org
Status: Assigned (was: Untriaged)
Adding kenrb to get your take on this. Does killing the renderer sound good?

Comment 8 by mcnee@chromium.org, Aug 4 2017

Blockedon: 752596
components/guest_view/browser/ doesn't have bad_message.h yet. I'll need to add that.

Comment 9 by kenrb@chromium.org, Aug 8 2017

Re: comment #7, yes killing the renderer and returning is reasonable in this situation.
Cc: mcnee@chromium.org
 Issue 752807  has been merged into this issue.
Status: Started (was: Assigned)
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 15 2017

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

commit 70e056c1d53900cce7f9047bbc796e6ecefda3b5
Author: Kevin McNee <mcnee@chromium.org>
Date: Tue Aug 15 21:04:27 2017

Kill a renderer if it sends an unexpected message before GVM creation.

If we receive a message from the renderer to
GuestViewMessageFilter::OnAttachToEmbedderFrame before the creation of
a GuestViewManager, then the renderer is misbehaving.

We now kill the renderer in this case.

Bug:  751401 
Change-Id: I2c2c0aa507a61ae9abb86f792e6fd710647fd9b5
Reviewed-on: https://chromium-review.googlesource.com/615474
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494542}
[modify] https://crrev.com/70e056c1d53900cce7f9047bbc796e6ecefda3b5/components/guest_view/browser/bad_message.h
[modify] https://crrev.com/70e056c1d53900cce7f9047bbc796e6ecefda3b5/components/guest_view/browser/guest_view_message_filter.cc
[modify] https://crrev.com/70e056c1d53900cce7f9047bbc796e6ecefda3b5/components/guest_view/browser/guest_view_message_filter.h
[modify] https://crrev.com/70e056c1d53900cce7f9047bbc796e6ecefda3b5/tools/metrics/histograms/enums.xml

Project Member

Comment 13 by ClusterFuzz, Aug 16 2017

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