Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Starred by 2 users
Status: Fixed
Owner:
Closed: Mar 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux, Windows, Chrome, Mac
Pri: 1
Type: Bug-Security

Blocking:
issue 701034



Sign in to add a comment
Security: UAF through chrome app
Reported by look.wan...@gmail.com, Feb 23 2017 Back to list
This is a variant of https://bugs.chromium.org/p/chromium/issues/detail?id=683523

VERSION
1) Chrome Version: latest build of chromium (64-bit)
   Operating System: Windows 10 x64

2) ASAN: asan-linux-stable-56.0.2924.87


ROOT CAUSE

https://cs.chromium.org/chromium/src/components/guest_view/renderer/guest_view_container.cc?l=190
void GuestViewContainer::OnHandleCallback(const IPC::Message& message) {
  // Handle the callback for the current request with a pending response.
  HandlePendingResponseCallback(message);
  // Perform the subsequent request if one exists.
  PerformPendingRequest();
}

When "HandlePendingResponseCallback" is called, js function "GuestViewImpl.prototype.handleCallback" is called:
(https://cs.chromium.org/chromium/src/extensions/renderer/resources/guest_view/guest_view.js?l=81)
GuestViewImpl.prototype.handleCallback = function(callback) {
  if (callback) {
    callback();
  }
  this.pendingAction = null;
  this.performNextAction();
};


Through setter of "pendingAction", we can delete current GuestViewContainer through removing iframe.




REPRODUCTION CASE

1) ASAN: Open chrome://extensions/, through "Develop Mode" load the app directory (app files are attached), then launch that app in chrome://apps/ tab.

==2523==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000019008 at pc 0x5632daabfee0 bp 0x7ffc5dcafbe0 sp 0x7ffc5dcafbd8
READ of size 1 at 0x60d000019008 thread T0 (chrome)
context mismatch in svga_sampler_view_destroy
context mismatch in svga_sampler_view_destroy
context mismatch in svga_sampler_view_destroy
context mismatch in svga_sampler_view_destroy
context mismatch in svga_sampler_view_destroy
    #0 0x5632daabfedf in guest_view::GuestViewContainer::PerformPendingRequest() components/guest_view/renderer/guest_view_container.cc:145:8



About fix:
I think we should set prototype of "GuestViewImpl.prototype" to be null to prevent user from messing with builtin js code

 
crash-poc.zip
9.4 KB Download
Labels: OS-Chrome OS-Linux OS-Mac OS-Windows
Reproduced.

SUMMARY: AddressSanitizer: heap-use-after-free (/Users/kerrnel/Downloads/asan-mac-release-452503/Chromium.app/Contents/Versions/58.0.3022.0/Chromium Framework.framework/Chromium Framework+0x19bf4e99) in guest_view::GuestViewContainer::PerformPendingRequest()
Components: Platform>Extensions
Labels: M-56 Security_Severity-High Security_Impact-Stable Pri-1
Owner: lazyboy@chromium.org
Status: Assigned
lazyboy@, can you please take a look or assign this to someone who can? Thank you.
Components: -Platform>Extensions Platform>Apps>BrowserTag
Owner: wjmaclean@chromium.org
Assigning to James.
Status: Started
Taking a look. Unlike the other UAF issue this one doesn't seem to cause an outright renderer crash in my initial repro attempts.
Cc: fsam...@chromium.org
Comment 6 by lfg@chromium.org, Feb 24 2017
Cc: lazyboy@chromium.org
Project Member Comment 7 by bugdroid1@chromium.org, Feb 24 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5934185d281ff83961832317620da5468e7cf703

commit 5934185d281ff83961832317620da5468e7cf703
Author: wjmaclean <wjmaclean@chromium.org>
Date: Fri Feb 24 22:59:01 2017

Don't allow GuestView JS objects to inherit global prototype.

Allowing objects like GuestViewImpl and GuestViewContainer to
inherit prototypes from the global JS object can allow arbitrary user
code to be attached to these objects, and potentially executed.
This CL prevents this by forcing the inherited __proto__ objects
to be null.

BUG= 695476 

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

[modify] https://crrev.com/5934185d281ff83961832317620da5468e7cf703/extensions/renderer/resources/guest_view/guest_view.js
[modify] https://crrev.com/5934185d281ff83961832317620da5468e7cf703/extensions/renderer/resources/guest_view/guest_view_container.js

Labels: Merge-Request-56
I'm going to go ahead and mark this as fixed, even though there's still a follow-on cleanup CL (that won't change any behaviour).

Is it too late to request merge to M56? This is a small, low-risk CL.
Status: Fixed
Project Member Comment 10 by sheriffbot@chromium.org, Mar 2
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-56 -Merge-Request-56 M-57 Merge-Request-57
Labels: reward-topanel
Project Member Comment 13 by sheriffbot@chromium.org, Mar 5
Labels: -Merge-Request-57 Hotlist-Merge-Review Merge-Review-57
This bug requires manual review: We are only 8 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
 awhalley@, is this good to take in for M57?
Cc: awhalley@chromium.org
govind@ - been in dev for a good 5 days, good for 57.
Labels: -Merge-Review-57 Merge-Aprpoved-57
Approving merge to M57 branch 2987 based on comment #16. Please merge ASAP (merge has to happen latest by 5:00 PM PT tomorrow, Monday, 03/06 so we can take it in for M57 Desktop Stable build cut). Thank you.
Labels: -Merge-Aprpoved-57 Merge-Approved-57
Labels: Release-0-57
Labels: -Release-0-57 Release-0-M57
Project Member Comment 21 by bugdroid1@chromium.org, Mar 6
Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7ea20cf7c8188d36a92be706a234afef673235f

commit c7ea20cf7c8188d36a92be706a234afef673235f
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Mon Mar 06 15:42:51 2017

Don't allow GuestView JS objects to inherit global prototype.

Allowing objects like GuestViewImpl and GuestViewContainer to
inherit prototypes from the global JS object can allow arbitrary user
code to be attached to these objects, and potentially executed.
This CL prevents this by forcing the inherited __proto__ objects
to be null.

BUG= 695476 

Review-Url: https://codereview.chromium.org/2712913005
Cr-Commit-Position: refs/heads/master@{#452976}
(cherry picked from commit 5934185d281ff83961832317620da5468e7cf703)

Review-Url: https://codereview.chromium.org/2730383002 .
Cr-Commit-Position: refs/branch-heads/2987@{#769}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/c7ea20cf7c8188d36a92be706a234afef673235f/extensions/renderer/resources/guest_view/guest_view.js
[modify] https://crrev.com/c7ea20cf7c8188d36a92be706a234afef673235f/extensions/renderer/resources/guest_view/guest_view_container.js

Labels: CVE-2017-5038
Labels: -Security_Severity-High Security_Severity-Medium
Blocking: 701034
Labels: -reward-topanel reward-unpaid reward-1000
Congratulations! The panel decided to award $1,000 for this bug.
Labels: -reward-unpaid reward-inprocess
Project Member Comment 28 by sheriffbot@chromium.org, Jun 8
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