New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 0 users
Status: Fixed
Owner:
Closed: Nov 2014
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
Use-of-uninitialized-value in aura::Window::GetNativeWindowProperty
Project Member Reported by ClusterFuzz, Oct 20 2014 Back to list
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6639045995134976

Fuzzer: Meacer_extension_apis
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  aura::Window::GetNativeWindowProperty
  ChromeViewsDelegate::OnBeforeWidgetInit
  views::Widget::Init
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=285497:285552

Minimized Testcase (4.36 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96cNPx5pKpHXyJz1qROWS6zXJnRB_G8H1WXOrC6IfW88E9YN7F-3LPynywJV6Cb1DjZHgtle_1PEnHUUxrxru9dHRC3fws414HdZwtSJAFBWxxDbST_J50VEqvpC5uES2UegOqtuSO0SeV_5dKTZlVvKsLcUg

Filer: inferno
 
Cc: earthdok@chromium.org glider@chromium.org kcc@chromium.org
Owner: marshall@chromium.org
Status: Assigned
Is this really a regression from this libc++ change

Author: marshall
Component: libc++
Changelist: https://chromium.googlesource.com/chromium/llvm-project/libcxx.git/+/e785ef19637f88c5e3e9926fabd8a64cd7eac49d
Time: Fri Apr 11 08:22:42 2014
File __tree is changed in this cl (and is part of stack frame #0, "__lower_bound"; frame #1, "find")
Minimum distance from crash line to modified line: 52. (file: __tree, crashed on: 2036, modified: 1984).
Project Member Comment 2 by ClusterFuzz, Oct 20 2014
Labels: Pri-1
Owner: ----
Status: Available
You probably want Marshall Clow (mclow.lists@gmail.com)
Cc: mclow.li...@gmail.com
Status: Assigned
+cc mclow.lists@gmail.com.
Cc: -mclow.li...@gmail.com sky@chromium.org
Status: Untriaged
No, this is a straight-up use-after-free. A rogue task accesses aura::Window after it's been destroyed. I'm not sure who's at fault here though.

Scott, could you please help triage this?
Comment 6 by glider@chromium.org, Oct 20 2014
For the record, the libc++ change is related to  issue 358707  and makes libc++ behave similarly to libstdc++. It's unlikely it could have introduced any bugs (but could perhaps uncover some existing ones).
Labels: -Security_Severity-Medium Security_Severity-High
Project Member Comment 8 by ClusterFuzz, Oct 22 2014
Labels: M-38
Project Member Comment 9 by ClusterFuzz, Oct 22 2014
Labels: Untriaged-1
Comment 10 by jww@chromium.org, Oct 22 2014
Owner: sky@chromium.org
Status: Assigned
Comment 11 by jww@chromium.org, Oct 22 2014
Labels: -Untriaged-1
Comment 12 by sky@chromium.org, Oct 22 2014
Owner: sashab@chromium.org
Looks as though ExtensionUninstallDialogViews is caching an aura::Window and not detecting if the aura::Window gets destroyed. Seems as though there is asynchronous logic here and ExtensionUninstallDialogViews needs to track the lifetime of the aura::Window.
Cc: sashab@chromium.org
Owner: pkotw...@chromium.org
I think this is being fixed by pkotwicz as evident in https://codereview.chromium.org/662073002/.
I wasn't aware of problems with the uninstall dialog. However, it makes sense for me to take this on.
What is a realistic milestone to fix this for? I can definately fix this for M-42 and perhaps for M-41. This bug is logged against M-38. I believe M-38 has shipped.
This is a high severity security vulnerability and cannot be punted. We always back-merge security patches. Please definitely target for M-39.
Cc: asargent@chromium.org
CCing asargent@ in case he has ideas how to fix this. (I have not looked into this issue yet. However, help earlier is better than later)

I am working on the issue of fixing use after frees during install (and now uninstall). The proper fix is large so would be suboptimal to backport. We will need to find a different way to fix this for M-39. I will look into this issue this week.


CL is up for review at https://codereview.chromium.org/697023002/
Status: Started
Project Member Comment 20 by bugdroid1@chromium.org, Nov 4 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea87b86d5bd1abe26636abfc19af28e755da7585

commit ea87b86d5bd1abe26636abfc19af28e755da7585
Author: pkotwicz <pkotwicz@chromium.org>
Date: Tue Nov 04 22:42:33 2014

Cancel uninstall if the uninstall dialog's parent window is destroyed prior to
the uninstall dialog showing.

BUG= 424999 ,  397396 
TEST=ExtensionUninstallDialogViewBrowserTest.TrackParentWindowDestruction

Review URL: https://codereview.chromium.org/697023002

Cr-Commit-Position: refs/heads/master@{#302691}

[modify] https://chromium.googlesource.com/chromium/src.git/+/ea87b86d5bd1abe26636abfc19af28e755da7585/chrome/browser/extensions/extension_uninstall_dialog.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/ea87b86d5bd1abe26636abfc19af28e755da7585/chrome/browser/extensions/extension_uninstall_dialog.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/ea87b86d5bd1abe26636abfc19af28e755da7585/chrome/browser/ui/cocoa/extensions/extension_uninstall_dialog_cocoa.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/ea87b86d5bd1abe26636abfc19af28e755da7585/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
[add] https://chromium.googlesource.com/chromium/src.git/+/ea87b86d5bd1abe26636abfc19af28e755da7585/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/ea87b86d5bd1abe26636abfc19af28e755da7585/chrome/chrome_tests.gypi

Status: Fixed
Project Member Comment 22 by ClusterFuzz, Nov 5 2014
Labels: -Restrict-View-SecurityTeam Merge-Triage M-39 M-40 Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

Your fix is very close to the branch point. After the branch happens, please make sure to check if your fix is in.

- Your friendly ClusterFuzz
Cc: benwells@chromium.org
I don't really think this justifies a merge. It's a really small bug, rare case. Thoughts benwells?
See comment #16.
Labels: -M-38 -Merge-Triage -M-40 Merge-Requested
Labels: -Merge-Requested Merge-Approved
merge approved for m39 branch 2171
Project Member Comment 27 by bugdroid1@chromium.org, Nov 11 2014
Labels: -Merge-Approved merge-merged-2171
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6919459fb7f674aaf05277ebe2fe12e746d4a3f9

commit 6919459fb7f674aaf05277ebe2fe12e746d4a3f9
Author: Peter Kotwicz <pkotwicz@google.com>
Date: Tue Nov 11 16:46:12 2014

[Merge] Cancel uninstall if the uninstall dialog's parent window is destroyed prior to the uninstall dialog showing.

BUG= 424999 ,  397396 
TEST=ExtensionUninstallDialogViewBrowserTest.TrackParentWindowDestruction
TBR=pkotwicz

Review URL: https://codereview.chromium.org/697023002

Review URL: https://codereview.chromium.org/711343003

Cr-Commit-Position: refs/branch-heads/2171@{#402}
Cr-Branched-From: 267aeeb8d85c8503a7fd12bd14654b8ea78d3974-refs/heads/master@{#297060}

[modify] https://chromium.googlesource.com/chromium/src.git/+/6919459fb7f674aaf05277ebe2fe12e746d4a3f9/chrome/browser/extensions/extension_uninstall_dialog.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/6919459fb7f674aaf05277ebe2fe12e746d4a3f9/chrome/browser/extensions/extension_uninstall_dialog.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/6919459fb7f674aaf05277ebe2fe12e746d4a3f9/chrome/browser/ui/cocoa/extensions/extension_uninstall_dialog_cocoa.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/6919459fb7f674aaf05277ebe2fe12e746d4a3f9/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
[add] https://chromium.googlesource.com/chromium/src.git/+/6919459fb7f674aaf05277ebe2fe12e746d4a3f9/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/6919459fb7f674aaf05277ebe2fe12e746d4a3f9/chrome/chrome_tests.gypi

Labels: Release-0-M39
Project Member Comment 29 by ClusterFuzz, Feb 11 2015
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 30 by sheriffbot@chromium.org, Oct 1 2016
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
Project Member Comment 31 by sheriffbot@chromium.org, Oct 2 2016
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
Labels: allpublic
Sign in to add a comment