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: Sep 2014
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
Heap-use-after-free in JavaObjectWeakGlobalRef::Assign
Project Member Reported by ClusterFuzz, Sep 12 2014 Back to list
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5607564067733504

Fuzzer: Kkania_chromebot
Job Type: Android_asan_chrome

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x90dd3064
Crash State:
  JavaObjectWeakGlobalRef::Assign
  content::AttachImeAdapter
  dvmPlatformInvoke
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv951BN71xqYnOLuZjYZK--7TKFFBSoQCj-4i56jODwK5G-YOpZruZ8NuHfN3l2Tzk0FxlN2WkG56N-Ymc4i80BgcTrxdk2ccDYkV1UTodQem1LHp0C8G07zN665O-gOTnDQYyQg78RC6nBC7aEufde3hICYH5g


Additional requirements: Requires Gestures

Additional requirements: Requires HTTP

Filer: inferno
 
Owner: feng@chromium.org
Status: Assigned
Comment 2 by feng@chromium.org, Sep 12 2014
=================================================================
==9145==ERROR: AddressSanitizer: heap-use-after-free on address 0x90dd3064 at pc 0x7ab52038 bp 0xbec9c340 sp 0xbec9c338
READ of size 4 at 0x90dd3064 thread T0 (oid.apps.chrome)
    #0 0x7ab52037 in JavaObjectWeakGlobalRef::Assign(JavaObjectWeakGlobalRef const&) src/base/android/jni_weak_ref.cc:63:7
    #1 0x87f2e4eb in content::AttachImeAdapter(_JNIEnv*, _jobject*, long long) src/content/browser/renderer_host/ime_adapter_android.cc:250:3
    #2 0x43f1dbcf in dvmPlatformInvoke dalvik/vm/arch/arm/CallEABI.S:258
    #3 0x42bc9dbf (<unknown module>)
0x90dd3064 is located 228 bytes inside of 1000-byte region [0x90dd2f80,0x90dd3368)
freed by thread T0 (oid.apps.chrome) here:
    #0 0x4011126b in operator delete(void*)
    #1 0x88042e93 in content::RenderWidgetHostImpl::Destroy() src/content/browser/renderer_host/render_widget_host_impl.cc:1344:5
    #2 0x8802dd47 in content::RenderViewHostImpl::Shutdown() src/content/browser/renderer_host/render_view_host_impl.cc:965:3
    #3 0x87ba0553 in content::FrameTree::UnregisterRenderFrameHost(content::RenderFrameHostImpl*) src/content/browser/frame_host/frame_tree.cc:300:7
    #4 0x87be1caf in content::RenderFrameHostImpl::~RenderFrameHostImpl()
    #5 0x87be1fbb in content::RenderFrameHostImpl::~RenderFrameHostImpl()
    #6 0x87c01a0b in content::RenderFrameHostManager::~RenderFrameHostManager() src/base/memory/scoped_ptr.h:137:5
    #7 0x87ba6247 in content::FrameTreeNode::~FrameTreeNode() src/content/browser/frame_host/frame_tree_node.cc:38:1
    #8 0x87b9cbef in content::FrameTree::~FrameTree() src/base/memory/scoped_ptr.h:137:5
    #9 0x87ba84cb in content::InterstitialPageImpl::~InterstitialPageImpl() src/content/browser/frame_host/interstitial_page_impl.cc:191:1
    #10 0x87ba85b3 in content::InterstitialPageImpl::~InterstitialPageImpl() src/content/browser/frame_host/interstitial_page_impl.cc:190:47
    #11 0x7ab7715b in base::debug::TaskAnnotator::RunTask(char const*, char const*, base::PendingTask const&) src/base/callback.h:401:12
    #12 0x7ac2398f in base::MessageLoop::RunTask(base::PendingTask const&) src/base/message_loop/message_loop.cc:446:3
    #13 0x7ac23eaf in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask const&) src/base/message_loop/message_loop.cc:456:5
    #14 0x7ac24c77 in base::MessageLoop::DoWork() src/base/message_loop/message_loop.cc:565:13
    #15 0x7ab18513 in DoRunLoopOnce(_JNIEnv*, _jobject*, long long, long long) src/base/message_loop/message_pump_android.cc:35:19
    #16 0x43f1dbcf in dvmPlatformInvoke dalvik/vm/arch/arm/CallEABI.S:258
    #17 0x42bc9e0f (<unknown module>)
previously allocated by thread T0 (oid.apps.chrome) here:
    #0 0x40110cbb in operator new(unsigned int)
    #1 0x882b4c2b in content::WebContentsViewAndroid::CreateViewForWidget(content::RenderWidgetHost*) src/content/browser/web_contents/web_contents_view_android.cc:141:3
    #2 0x87bade6b in content::InterstitialPageImpl::CreateWebContentsView()
    #3 0x87ba8e27 in content::InterstitialPageImpl::Show() src/content/browser/frame_host/interstitial_page_impl.cc:249:3
    #4 0x88219033 in content::SSLPolicy::OnCertErrorInternal(content::SSLCertErrorHandler*, int) src/content/browser/ssl/ssl_policy.cc:198:3
    #5 0x88218c17 in content::SSLPolicy::OnCertError(content::SSLCertErrorHandler*) src/content/browser/ssl/ssl_policy.cc:65:7
    #6 0x7ab7715b in base::debug::TaskAnnotator::RunTask(char const*, char const*, base::PendingTask const&) src/base/callback.h:401:12
    #7 0x7ac2398f in base::MessageLoop::RunTask(base::PendingTask const&) src/base/message_loop/message_loop.cc:446:3
    #8 0x7ac23eaf in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask const&) src/base/message_loop/message_loop.cc:456:5
    #9 0x7ac24c77 in base::MessageLoop::DoWork() src/base/message_loop/message_loop.cc:565:13
    #10 0x7ab18513 in DoRunLoopOnce(_JNIEnv*, _jobject*, long long, long long) src/base/message_loop/message_pump_android.cc:35:19
    #11 0x43f1dbcf in dvmPlatformInvoke dalvik/vm/arch/arm/CallEABI.S:258
    #12 0x42bc9e0f (<unknown module>)
SUMMARY: AddressSanitizer: heap-use-after-free ??:0 ??
Comment 3 by feng@chromium.org, Sep 12 2014
Cc: feng@chromium.org
Owner: aurimas@chromium.org
It is a user-after-free. From the code, ImeAdapterAndroid::~ImeAdapterAndroid calls Java side to detach the native code. Allocation, deallocation, and crash all happened on the same thread, so might be another ImeAdapterAndroid object holds the reference to the deleted native object?
Comment 4 by tsepez@chromium.org, Sep 12 2014
Determining the affected versions is difficult at present because this doesn't reproduce reliably for CF.  Once the issue is understood, it should be possible to figure out how far back it goes.
Project Member Comment 5 by ClusterFuzz, Sep 12 2014
Labels: Pri-1
Project Member Comment 6 by ClusterFuzz, Sep 16 2014
Labels: Missing_Impact-1
Comment 7 by jww@chromium.org, Sep 19 2014
Labels: -Missing_Impact-1 Security_Impact-Head
Comment 8 by jww@chromium.org, Sep 19 2014
Labels: -Security_Impact-Head Security_Impact-Stable
The impact is unclear so marking as stable. If you have reason to believe otherwise, please outline why.
Project Member Comment 9 by ClusterFuzz, Sep 19 2014
Labels: M-37
Project Member Comment 10 by ClusterFuzz, Sep 20 2014
Labels: Nag
aurimas@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Cc: aurimas@chromium.org
Owner: bcwh...@chromium.org
bcwhite: do you have cycles to look at this?
Viewing this page on Android has an interstitial page warning about the SSL certificate.  The crash report seems to indicate it has something to do with that.
My current working theory is that there is some event queued when the Interstitial is exiting.  If that event fires before the page is destroyed, no problem, but if it comes in after then the RenderViewHostImpl has already been destroyed and !poof!.  RVHI::Destroy() does a "delete this" so is not properly managed; seems a bad methodology for an object that is held by other objects.

Reproducing this is difficult.  I managed once (my first try) but not again with over a dozen attempts.
One possibility is the DelayedDismissInput runnable that holds a reference to the native.  Callbacks to instances of that class get removed by the detach() call but if it still gets called for some reason, or perhaps is in the process of being called due to threading or event queues, that could cause this.

I can put an additional safety check on this but without a reliable way to reproduce the issue, I can't tell if it's actually fixing anything.
This is a one-time crasher, there is no other way, than to try a speculative fix and then we can reopen bug if we see the same stack again.
Project Member Comment 16 by bugdroid1@chromium.org, Sep 26 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afdaf434f9435722f239de0954df3618156ae0a0

commit afdaf434f9435722f239de0954df3618156ae0a0
Author: bcwhite <bcwhite@chromium.org>
Date: Fri Sep 26 17:46:47 2014

Increase safety of delayed-keyboard-dismiss to not call destroyed object.

BUG= 413744 

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

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

[modify] https://chromium.googlesource.com/chromium/src.git/+/afdaf434f9435722f239de0954df3618156ae0a0/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java

Status: Fixed
Assuming fixed.  Re-open if it happens again.

Project Member Comment 18 by ClusterFuzz, Sep 26 2014
Labels: -Restrict-View-SecurityTeam Merge-Triage M-39 M-38 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
Labels: -M-37 -M-39 Merge-Requested
No problems have been reported over the past week so though we can't be sure it's fixed the reported problem, it's at least not causing any other problems.  It landed in time for the M39 beta cut.  Shall we merge it into M38?
Yes.
Labels: -Merge-Requested Aprp
Labels: -Aprp Merge-Approved
Labels: -Merge-Approved Merge-Merged
Labels: -Merge-Triage -M-38 Merge-Requested M-39
Is a M-39 merge needed for this ? Was this released in M38 ?
Labels: -M-39
M39 branched at 297060; the above change landed at 296973.  No merge required for M38.
Labels: -Nag -Merge-Requested Release-0-M39
Project Member Comment 27 by ClusterFuzz, Jan 2 2015
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 28 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 29 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