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

Issue 461191 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Blocked on:
issue 461427



Sign in to add a comment

Security: UNKNOWN in RenderFrameImpl::OnMessageReceived

Reported by chromium...@gmail.com, Feb 24 2015

Issue description

VERSION
Chrome Version: 43.0.2313.0 canary
Operating System: Windows 7

REPRODUCTION CASE
Actually this issue needs several attempts for repro the crash, I uploaded a video for see how I repro it 
but I'm looking for another way to repro it easily.

Crash ID: a20a8abec387eccc
          206d3cf668db709e 
          c7967da3ee8b4998 


eax=88ef4125 ebx=0890b400 ecx=2541e630 edx=001cf5a4 esi=0738eb50 edi=0890b400
eip=0f458e67 esp=001cf2e0 ebp=001cf62c iopl=0         nv up ei ng nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010286
chrome_child!content::RenderFrameImpl::OnMessageReceived+0x27:
0f458e67 ff5064          call    dword ptr [eax+64h]  ds:0023:88ef4189=????????
0:000> k
  *** Stack trace for last set context - .thread/.cxr resets it
ChildEBP RetAddr  
001cf62c 0f414a39 chrome_child!content::RenderFrameImpl::OnMessageReceived+0x27 [c:\b\build\slave\win\build\src\content\renderer\render_frame_impl.cc @ 950]
001cf63c 0f414a11 chrome_child!content::MessageRouter::RouteMessage+0x24 [c:\b\build\slave\win\build\src\content\common\message_router.cc @ 55]
001cf648 0f3870a6 chrome_child!content::MessageRouter::OnMessageReceived+0x1d [c:\b\build\slave\win\build\src\content\common\message_router.cc @ 47]
001cf6d8 0f386fe3 chrome_child!content::ChildThreadImpl::OnMessageReceived+0xa3 [c:\b\build\slave\win\build\src\content\child\child_thread_impl.cc @ 545]
001cf70c 0f386f48 chrome_child!IPC::ChannelProxy::Context::OnDispatchMessage+0x98 [c:\b\build\slave\win\build\src\ipc\ipc_channel_proxy.cc @ 283]
001cf71c 0f385a56 chrome_child!base::internal::Invoker<IndexSequence<0,1>,base::internal::BindState<base::internal::RunnableAdapter<void (__thiscall content::InputEventFilter::*)(IPC::Message const &)>,void __cdecl(content::InputEventFilter *,IPC::Message const &),base::internal::TypeList<content::InputEventFilter *,IPC::Message> >,base::internal::TypeList<base::internal::UnwrapTraits<content::InputEventFilter *>,base::internal::UnwrapTraits<IPC::Message> >,base::internal::InvokeHelper<0,void,base::internal::RunnableAdapter<void (__thiscall content::InputEventFilter::*)(IPC::Message const &)>,base::internal::TypeList<content::InputEventFilter * const &,IPC::Message const &> >,void __cdecl(void)>::Run+0x20 [c:\b\build\slave\win\build\src\base\bind_internal.h @ 346]
001cf794 0f3f29fe chrome_child!base::debug::TaskAnnotator::RunTask+0x1b7 [c:\b\build\slave\win\build\src\base\debug\task_annotator.cc @ 63]
001cf7e0 0f3f2554 chrome_child!content::TaskQueueManager::ProcessTaskFromWorkQueue+0x4c [c:\b\build\slave\win\build\src\content\renderer\scheduler\task_queue_manager.cc @ 419]
001cf804 0f3f24c2 chrome_child!content::TaskQueueManager::DoWork+0x8e [c:\b\build\slave\win\build\src\content\renderer\scheduler\task_queue_manager.cc @ 389]
001cf818 0f3f2483 chrome_child!base::internal::InvokeHelper<1,void,base::internal::RunnableAdapter<void (__thiscall content::TaskQueueManager::*)(bool)>,base::internal::TypeList<base::WeakPtr<content::TaskQueueManager> const &,bool const &> >::MakeItSo+0x3a [c:\b\build\slave\win\build\src\base\bind_internal.h @ 303]
001cf82c 0f385a56 chrome_child!base::internal::Invoker<IndexSequence<0,1>,base::internal::BindState<base::internal::RunnableAdapter<void (__thiscall content::TaskQueueManager::*)(bool)>,void __cdecl(content::TaskQueueManager *,bool),base::internal::TypeList<base::WeakPtr<content::TaskQueueManager>,bool> >,base::internal::TypeList<base::internal::UnwrapTraits<base::WeakPtr<content::TaskQueueManager> >,base::internal::UnwrapTraits<bool> >,base::internal::InvokeHelper<1,void,base::internal::RunnableAdapter<void (__thiscall content::TaskQueueManager::*)(bool)>,base::internal::TypeList<base::WeakPtr<content::TaskQueueManager> const &,bool const &> >,void __cdecl(void)>::Run+0x1e [c:\b\build\slave\win\build\src\base\bind_internal.h @ 346]
001cf8a4 0f3856db chrome_child!base::debug::TaskAnnotator::RunTask+0x1b7 [c:\b\build\slave\win\build\src\base\debug\task_annotator.cc @ 63]
001cf8dc 0f38552d chrome_child!base::MessageLoop::RunTask+0xed [c:\b\build\slave\win\build\src\base\message_loop\message_loop.cc @ 451]
001cf9e8 0f387a06 chrome_child!base::MessageLoop::DoWork+0x2c1 [c:\b\build\slave\win\build\src\base\message_loop\message_loop.cc @ 571]
001cfa14 0f3851b2 chrome_child!base::MessagePumpDefault::Run+0xc7 [c:\b\build\slave\win\build\src\base\message_loop\message_pump_default.cc @ 33]
001cfa38 0f3850ba chrome_child!base::MessageLoop::RunHandler+0x65 [c:\b\build\slave\win\build\src\base\message_loop\message_loop.cc @ 415]
001cfa60 0f386624 chrome_child!base::RunLoop::Run+0x88 [c:\b\build\slave\win\build\src\base\run_loop.cc @ 56]
001cfa84 0f3d8e5c chrome_child!base::MessageLoop::Run+0x16 [c:\b\build\slave\win\build\src\base\message_loop\message_loop.cc @ 308]
001cfc24 0f37e265 chrome_child!content::RendererMain+0x277 [c:\b\build\slave\win\build\src\content\renderer\renderer_main.cc @ 221]
001cfc38 0f37e1e1 chrome_child!content::RunNamedProcessTypeMain+0x61 [c:\b\build\slave\win\build\src\content\app\content_main_runner.cc @ 385]
 
Chrome-last.dmp
284 KB Download
video.mp4
489 KB Download
testcase.html
1.1 KB View Download
Blockedon: chromium:461427

Comment 3 Deleted

Project Member

Comment 4 by ClusterFuzz, Feb 27 2015

Labels: Untriaged-1
Reproduced on stable channel (40.0.2214.115 m).

Still trying to figure it out. Here is crash ID:

Crash ID 684cc87c99c55601


Labels: -Untriaged-1 Cr-Blink-WebRTC Security_Impact-Stable Security_Severity-High OS-All
Owner: phoglund@chromium.org
Status: Assigned
I see navigator.webkitGetUserMedia, and your repro shows camera pop-us. so, something related to webrtc. Assigning to Patrick for help in triage and finding owner.
I created a new testcase because I couldn't repro this with using that testcase in #1 this crash seems like needs some iframe contains ads for keep pages loading as in screenshot-1.png. I uploaded a video too see how I repro this.
steps.mp4
1.0 MB Download
testcase.html
1.1 KB View Download
Project Member

Comment 8 by ClusterFuzz, Mar 2 2015

Labels: M-41 Pri-1
Cc: magjed@chromium.org tnakamura@chromium.org tommi@chromium.org perkj@chromium.org phoglund@chromium.org
Owner: ----
Status: Available
chromium.khalil: What is it you're doing in the beginning when you open all those tabs? Are the multiple tabs necessary to repro?

I'll CC in some WebRTC people, but I'm not sure this is a pure WebRTC crash. The problem might be in the borderlands between WebRTC and the rest of the browser. RenderFrameImpl does contain references to web media player which plays back WebRTC video and audio (I think), but it never gets used in this case because we can see in the testcase that we never play the media stream (on gUM success, close the window and throw away the stream).

The crash happens in OnMessage: is this a crash in the IPC framework? Abhishek, can you CC in some IPC people or people working on the general browser to help understand this?
Cc: nasko@chromium.org creis@chromium.org
Patrick, When I open those tabs I wait til see some tabs are ready (are not loading) as in screenshot.png and go to some of them than keep clicking on the page.
screenshot.png
52.0 KB View Download
I have made this video I think is more clarity in steps.
issue 461191.mp4
1.5 MB Download
According to the testcase code in #7, each tab that is created includes this onclick handler:

    i.onclick = function() {
      setTimeout(function() {  
                navigator.webkitGetUserMedia({video: true}, function(){window.close()}, function(){window.close()});
                window.close()
           }, 200);

Perhaps because the gUM call is asynchronous, I've only seen the gUM prompt appear once, and I wasn't quick enough to click the infobar. I may also be having problems reproducing this because my setup differs from the originally reported M43 crash reports in these ways:
a) crash reports - from a 32 bit machine, while I was testing with 64 bit binaries
b) uploaded videos indicate use of http server, while I was testing over https. This difference is important because https should remember gUM responses to requests from the same domain.

Even if I get a repro, I'm not clear that WebRTC is the cause. 

chromium.khalil@ - can you see if you can repro with nearly the same testcase code, except swap out the getUserMedia call with something else that triggers an infobar, such as a geolocation request?
I have tried with using SpeechRecognition API and geolocation but I wasn't able to repro.
Project Member

Comment 15 by ClusterFuzz, Mar 5 2015

Labels: Missing_Owner-2

Comment 16 by jln@chromium.org, Mar 5 2015

Owner: tnakamura@chromium.org
Status: Assigned
tnakamura@, assigning to you for better tracking, but re-assign if needed.
Cc: nick@chromium.org jam@chromium.org
Owner: ----
Status: Untriaged
We are unsure on how to proceed. Ted can't repro manually and it's probably at best tangentially related to WebRTC. nasko, creis, do you have any IPC wisdom to impart here? 

Appears the crash is in content/renderer/render_frame_impl.cc. Back at the time for Chrome 43.0.2313.0 (base commit 2b5fc4b664d47828d4e999a0fb17b22038318864) this was the code around line 950:

bool RenderFrameImpl::OnMessageReceived(const IPC::Message& msg) {
  // TODO(kenrb): document() should not be null, but as a transitional step
  // we have RenderFrameProxy 'wrapping' a RenderFrameImpl, passing messages
  // to this method. This happens for a top-level remote frame, where a
  // document-less RenderFrame is replaced by a RenderFrameProxy but kept
  // around and is still able to receive messages.
  if (!frame_->document().isNull())
    GetContentClient()->SetActiveURL(frame_->document().url());

  ObserverListBase<RenderFrameObserver>::Iterator it(observers_);
  RenderFrameObserver* observer;
  while ((observer = it.GetNext()) != NULL) {
    if (observer->OnMessageReceived(msg))
      return true;
  }

This line is the one that blows up:

if (!frame_->document().isNull())

, so I guess either frame_ or frame_->document() is junk.

cc:ing jam and nick who are in the blame list around that area. Can you help us understand how this can happen? What do we need to do to process? This is apparently quite hard to repro.

Also, is it really high priority, impacts stable, security and so on?
Project Member

Comment 18 by ClusterFuzz, Mar 11 2015

Labels: Untriaged-4
Project Member

Comment 19 by ClusterFuzz, Mar 11 2015

Labels: -Untriaged-4 Untriaged-5

Comment 20 by wfh@chromium.org, Mar 12 2015

Labels: -Missing_Owner-2 -Untriaged-5
Owner: creis@chromium.org
possible renderframe lifetime issue?  nasko or creis can you take a look at this?

Comment 21 by creis@chromium.org, Mar 13 2015

Cc: lfg@chromium.org
Owner: dcheng@chromium.org
Status: Assigned
Taking a quick look to triage this.  The crashes date back to about Chrome 36.

It boils down to a WebLocalFrame getting deleted before RenderFrameImpl.  Daniel (or Lucas): what's the expected order of deletion for WebLocalFrame and RenderFrameImpl?  Is there a way we can at least null out the frame_ pointer in RenderFrameImpl rather than leaving it dangling and leading to this UaF?

Side note: Ken's TODO and the isNull() call (see comment #17) are stale after my r305276.  I'll remove them, but that will just postpone the UaF to the following line's frame_->document().url() call.

Comment 22 by creis@chromium.org, Mar 13 2015

Owner: creis@chromium.org
Yikes.  For main frames, we delete the WebLocalFrameImpl in RenderFrameImpl::frameDetached(), when it calls WebLocalFrame::Close().  But the RenderFrameImpl itself doesn't get deleted until an entirely separate event, via RenderWidget::Release (which deletes both the RenderView and RenderFrame).  In the meantime, RenderFrameImpl::is_detaching_ is true, but we have a dangling frame_ pointer and no one (except Send) checks it.

This means that arbitrary methods can be called when frame_ is bogus.

RenderViewImpl seems to take care of this by nulling out its webview() (actually, RenderWidget does this), and then having null checks everywhere on the accesses to webview().

Unfortunate, but we need to do the same in RenderFrameImpl.  frameDetached should null out frame_ and everyone needs to null check it.

I'll do this first thing on Monday unless someone beats me to it.
Project Member

Comment 23 by ClusterFuzz, Mar 14 2015

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5761100646187008

Fuzzer: Aohelin_ni
Job Type: Linux_asan_chrome_mp

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x611000064040
Crash State:
  content::RenderFrameImpl::OnMessageReceived
  content::MessageRouter::RouteMessage
  content::MessageRouter::OnMessageReceived
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95jojrwBlyTwfqwzEMKg1uZeGVXiNls03N6jsVMcbFvcEF8Gr0fHKCxCra3fOGv78I67m6laz2pC40dC1n91Idv-u_-lIVHpQA5h9_2Q0FgTqEX_3awnJGGMwhmVMnlwGqDFwuzSqZ6JMs_YPxwTA-zZREs7UGHJFytPH1tk-biaK2x360


Filer: inferno
Cc: aohe...@gmail.com
Charlie, i think c#23 is the same bug and we have a nice uaf stack if you need ?

Comment 25 by creis@chromium.org, Mar 16 2015

Status: Started
Thanks.  I've got a fix with test up for review:
https://codereview.chromium.org/1007123003/

Comment 26 by creis@chromium.org, Mar 16 2015

Status: Fixed
The automated commit email hasn't shown up, but the fix for this landed in r320773.  Should be gone from tomorrow's canary.

Once that has a chance to bake, we can see about merging the fix.
Thanks Charlie! Tomorrow I will test it on canary.
Project Member

Comment 28 by ClusterFuzz, Mar 17 2015

Labels: -Restrict-View-SecurityTeam M-42 Merge-Triage 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 friendly ClusterFuzz
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 17 2015

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

commit cfaa4468f3394995a9f1565104ee2743a30d58e0
Author: creis <creis@chromium.org>
Date: Mon Mar 16 19:27:18 2015

Clear RenderFrameImpl::frame_ pointer after deleting it.

Also avoid dereferencing it in OnMessageReceived after deletion.

BUG= 461191 
TEST=No more crashes in RenderFrameImpl::OnMessageReceived

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

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

[modify] http://crrev.com/cfaa4468f3394995a9f1565104ee2743a30d58e0/content/renderer/render_frame_impl.cc
[modify] http://crrev.com/cfaa4468f3394995a9f1565104ee2743a30d58e0/content/renderer/render_frame_impl.h
[modify] http://crrev.com/cfaa4468f3394995a9f1565104ee2743a30d58e0/content/renderer/render_view_browsertest.cc
[modify] http://crrev.com/cfaa4468f3394995a9f1565104ee2743a30d58e0/content/renderer/render_view_impl.h

Comment 30 by creis@chromium.org, Mar 18 2015

Labels: -Merge-Triage Merge-Requested
It looks like this is doing well.  I don't see any RenderFrameImpl::OnMessageReceived crashes as of 43.0.2335.0.

Shall we merge it to M42 (and later M41)?

Comment 31 by amin...@google.com, Mar 18 2015

Labels: -Merge-Requested Merge-Review Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M41), manual review required.

Comment 32 by amin...@google.com, Mar 18 2015

Labels: Merge-Approved Hotlist-Merge-Approved
Approved for M42 (branch: 2311)

Comment 33 by creis@chromium.org, Mar 18 2015

Great.  I'll do it first thing tomorrow, since I need to head out early today.
Project Member

Comment 34 by bugdroid1@chromium.org, Mar 19 2015

Labels: -Merge-Approved merge-merged-2311
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b92260b108fc4b79361e9ea30ad903fa8359e338

commit b92260b108fc4b79361e9ea30ad903fa8359e338
Author: creis <creis@chromium.org>
Date: Thu Mar 19 16:19:58 2015

Clear RenderFrameImpl::frame_ pointer after deleting it.

Also avoid dereferencing it in OnMessageReceived after deletion.

NOTRY=true
TBR=nasko
BUG= 461191 
TEST=No more crashes in RenderFrameImpl::OnMessageReceived

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

Cr-Commit-Position: refs/heads/master@{#320773}
(cherry picked from commit cfaa4468f3394995a9f1565104ee2743a30d58e0)

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

Cr-Commit-Position: refs/branch-heads/2311@{#284}
Cr-Branched-From: 09b7de5dd7254947cd4306de907274fa63373d48-refs/heads/master@{#317474}

[modify] http://crrev.com/b92260b108fc4b79361e9ea30ad903fa8359e338/content/renderer/render_frame_impl.cc
[modify] http://crrev.com/b92260b108fc4b79361e9ea30ad903fa8359e338/content/renderer/render_frame_impl.h
[modify] http://crrev.com/b92260b108fc4b79361e9ea30ad903fa8359e338/content/renderer/render_view_browsertest.cc
[modify] http://crrev.com/b92260b108fc4b79361e9ea30ad903fa8359e338/content/renderer/render_view_impl.h

Comment 35 by creis@chromium.org, Mar 19 2015

Labels: Merge-Requested
Looks like the merge to M42 cleared the beta builders.  Should I merge to M41 as well, or wait for M42 to bake?
Labels: -Merge-Review -Hotlist-Merge-Review -Merge-Requested Merge-Approved
Merge approved for m41 branch 2272.
Project Member

Comment 37 by bugdroid1@chromium.org, Mar 20 2015

Labels: -Merge-Approved merge-merged-2272
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e176af4a53e7d4483e5dda125d7f77f2818e697c

commit e176af4a53e7d4483e5dda125d7f77f2818e697c
Author: creis <creis@chromium.org>
Date: Fri Mar 20 16:49:01 2015

Clear RenderFrameImpl::frame_ pointer after deleting it.

Also avoid dereferencing it in OnMessageReceived after deletion.

NOTRY=true
TBR=nasko@chromium.org
BUG= 461191 
TEST=No more crashes in RenderFrameImpl::OnMessageReceived

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

Cr-Commit-Position: refs/heads/master@{#320773}
(cherry picked from commit cfaa4468f3394995a9f1565104ee2743a30d58e0)

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

Cr-Commit-Position: refs/branch-heads/2272@{#445}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}

[modify] http://crrev.com/e176af4a53e7d4483e5dda125d7f77f2818e697c/content/renderer/render_frame_impl.cc
[modify] http://crrev.com/e176af4a53e7d4483e5dda125d7f77f2818e697c/content/renderer/render_frame_impl.h
[modify] http://crrev.com/e176af4a53e7d4483e5dda125d7f77f2818e697c/content/renderer/render_view_browsertest.cc
[modify] http://crrev.com/e176af4a53e7d4483e5dda125d7f77f2818e697c/content/renderer/render_view_impl.h

Project Member

Comment 38 by bugdroid1@chromium.org, Mar 31 2015

Labels: Release-2-M41 reward-topanel
chromium.khalil@, have you been able to verify that this is fixed in canary and/or any of the builds where the fix was merged into? It looks like the merges were approved based on the lack of crashes in canary (#30), which is fine, but I also want to make sure the fix looks good to you. :)
I am unable again to reproduce a crash with 44.0.2362.0 canary.
Cc: timwillis@chromium.org
Labels: -reward-topanel reward-3000 reward-unpaid CVE-2015-1237
Congratulations - $3000 for this report.

Notes from panel: We're paying a higher amount here because we think that may be scenarios where this could possibly result in a sandbox escape.
Labels: -reward-unpaid reward-inprocess
Project Member

Comment 44 by ClusterFuzz, Jun 23 2015

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Labels: -reward-inprocess
Processing via our e-payment system can take up to two weeks, but the reward should be on its way to you. Thanks again for your help!
Project Member

Comment 46 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 47 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
Labels: CVE_description-submitted

Sign in to add a comment