New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
Closed: May 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Security



Sign in to add a comment
Security: UAF through chrome app
Reported by look.wan...@gmail.com, May 4 Back to list
UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce the problem:
Open chrome://extensions/, use "Develop Mode" option to load the app directory (app files are attached), then launch that app in chrome://apps/ tab.

==27914==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000048068 at pc 0x563e99fe8870 bp 0x7ffdd9c34a20 sp 0x7ffdd9c34a18
READ of size 1 at 0x60d000048068 thread T0 (chrome)
#0 0x563e99fe886f in guest_view::GuestViewContainer::PerformPendingRequest() components/guest_view/renderer/guest_view_container.cc:146:8
    #1 0x563e99fe8f34 in OnHandleCallback components/guest_view/renderer/guest_view_container.cc:192:3
    #2 0x563e99fe8f34 in guest_view::GuestViewContainer::OnMessageReceived(IPC::Message const&) components/guest_view/renderer/guest_view_container.cc:203

What is the expected behavior?

What went wrong?
This is a variant of https://bugs.chromium.org/p/chromium/issues/detail?id=705941

We can redefine "shift" function to execute our js code:
https://cs.chromium.org/chromium/src/extensions/renderer/resources/guest_view/guest_view.js?l=97

this.pendingAction = this.actionQueue.shift();

Fix:

1)  use $Array.shift() like "push"
2)  One more important thing(I already said last time):

Why not use a weak pointer of "this" to check if "this" is deleted?
Check the pointer after "HandlePendingResponseCallback(message)" to avoid more potential UAFs here

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

Did this work before? N/A 

Chrome version: asan-linux-release-469194  Channel: n/a
OS Version: 10.0
Flash Version: Shockwave Flash 25.0 r0
 
guestview.zip
9.5 KB Download
Components: Platform>Apps>BrowserTag
Labels: Security_Severity-Medium Security_Impact-Stable
Owner: paulmeyer@chromium.org
Status: Assigned
paulmeyer, PTAL.

Assuming this impacts stable out of caution, but please correct this label if it's incorrect.
Project Member Comment 3 by sheriffbot@chromium.org, May 5
Labels: M-59
Project Member Comment 4 by sheriffbot@chromium.org, May 5
Labels: -Pri-2 Pri-1
Project Member Comment 5 by sheriffbot@chromium.org, May 18
paulmeyer: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

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 6 by bugdroid1@chromium.org, May 25
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c3e0f34012c116cbfefa64683ac33f5ebe621618

commit c3e0f34012c116cbfefa64683ac33f5ebe621618
Author: paulmeyer <paulmeyer@chromium.org>
Date: Thu May 25 19:57:25 2017

Fix for GuestView exposure through shift().

This patch converts all uses of shift() in the GuestView JavaScript
objects to use $Array.shift() instead. This will prevent GuestView
JavaScript objects from being exposed via overriding shift().

This patch also utilizes a weak pointer in
GuestViewContainer::OnHandleCallback() to ensure that |this| has not been
deleted in the callback, thus preventing further use-after-free bugs of
this type.

BUG= 718292 

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

[modify] https://crrev.com/c3e0f34012c116cbfefa64683ac33f5ebe621618/extensions/renderer/resources/guest_view/extension_view/extension_view.js
[modify] https://crrev.com/c3e0f34012c116cbfefa64683ac33f5ebe621618/extensions/renderer/resources/guest_view/guest_view.js
[modify] https://crrev.com/c3e0f34012c116cbfefa64683ac33f5ebe621618/extensions/renderer/safe_builtins.cc

Project Member Comment 7 by bugdroid1@chromium.org, May 25
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe556ec56788b6e25f0b38617c95d73a0317c0c3

commit fe556ec56788b6e25f0b38617c95d73a0317c0c3
Author: paulmeyer <paulmeyer@chromium.org>
Date: Thu May 25 20:39:43 2017

Prevent UAFs in GuestView objects.

This patch utilizes a weak pointer in
GuestViewContainer::OnHandleCallback() to ensure that |this| has not
been deleted in the callback, thus preventing further use-after-free
bugs of this type.

BUG= 718292 

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

[modify] https://crrev.com/fe556ec56788b6e25f0b38617c95d73a0317c0c3/components/guest_view/renderer/guest_view_container.cc

Status: Fixed
Project Member Comment 9 by sheriffbot@chromium.org, May 27
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: reward-topanel
Labels: -M-59 M-60
Labels: -reward-topanel reward-unpaid reward-2000
Nice one! The panel decided to award $2,000 for this report!
Labels: -reward-unpaid reward-inprocess
Project Member Comment 15 by sheriffbot@chromium.org, Jun 9
Labels: Merge-Request-60
Project Member Comment 16 by sheriffbot@chromium.org, Jun 9
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 17 by sheriffbot@chromium.org, Jun 12
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
Labels: -Merge-Approved-60
Removing merge label as this was checked in before branch point.
Labels: Release-0-M60
Labels: CVE-2017-5100
Project Member Comment 21 by sheriffbot@chromium.org, Sep 2
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