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

Issue 747105 link

Starred by 1 user
Status: Fixed
Owner:
lazyboy@chromium.org
Closed: Jul 2017
Cc:
karandeepb@chromium.org
█ msrchandra@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in extensions::ExtensionFunctionDispatcher::UIThreadWorkerResponseCallbackWrapper::

Project Member Reported by ClusterFuzz, Jul 20 2017 
Detailed report: https://clusterfuzz.com/testcase?key=6025695146016768

Fuzzer: ipc_fuzzer_gen
Job Type: linux_asan_chrome_ipc
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  extensions::ExtensionFunctionDispatcher::UIThreadWorkerResponseCallbackWrapper::
  extensions::ExtensionFunctionDispatcher::Dispatch
  bool IPC::MessageT<ExtensionHostMsg_RequestWorker_Meta, std::__1::tuple<Extensio
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_ipc&range=488146:488166

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6025695146016768


Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.

Comment 1 by msrchandra@chromium.org, Jul 21 2017

Cc: msrchandra@chromium.org
Labels: Test-Predator-Correct-CLs
Owner: sky@chromium.org
Status: Assigned (was: Untriaged)
Assigning to concern owner from Predator results --
Regression information is not available. The result is the blame information. 

Author: sky@chromium.org
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/5924a0b4c58a2f889752a6c76256895a62b31eb0
Time: Fri Apr 27 17:02:28 2012
The CL last changed line 32 of file scoped_observer.h, which is stack frame 0. 

Author: lazyboy
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/ee4adef0b70f13cb80a886ef639fc910c0b5ce6f
Time: Tue May 24 00:55:16 2016
The CL last changed line 182 of file extension_function_dispatcher.cc, which is stack frame 1. 

Author: lazyboy
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/ee4adef0b70f13cb80a886ef639fc910c0b5ce6f
Time: Tue May 24 00:55:16 2016
The CL last changed line 387 of file extension_function_dispatcher.cc, which is stack frame 2. 

Author: tzik
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/f7c47573b7686c59723f3b7da6d69f1ec494f23b
Time: Wed Apr 05 21:45:03 2017
The CL last changed line 77 of file tuple.h, which is stack frame 3. 

Author: tzik
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/2387a82551340c8b9e3e00ed3e7329fbeda22ba9
Time: Thu Aug 25 13:57:14 2016
The CL last changed line 84 of file tuple.h, which is stack frame 4. 

Author: mdempsky
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/8a5190449d48e06efa581390426dfa3bb6750f4c
Time: Tue Feb 09 05:41:47 2016
The CL last changed line 26 of file ipc_message_templates.h, which is stack frame 5. 

Author: mdempsky
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/8a5190449d48e06efa581390426dfa3bb6750f4c
Time: Tue Feb 09 05:41:47 2016
The CL last changed line 121 of file ipc_message_templates.h, which is stack frame 6.

@sky -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.

Comment 2 by sky@chromium.org, Jul 21 2017

Owner: lazyboy@chromium.org
Looks to me like content::RenderProcessHost::FromID(render_process_id_) is returning null. I'm not familiar enough with this code to know under what conditions that would happen. Passing to lazyboy.

Comment 3 by lazyboy@chromium.org, Jul 21 2017

Cc: karandeepb@chromium.org
Status: Started (was: Assigned)
My assumption that render_process_id_ will point to a live RenderProcessHost when we receive a message from BrowserFilter is certainly wrong. e.g. I see a lots of !render_process_host bailouts in ExtensionMessageFilter:

https://cs.chromium.org/chromium/src/extensions/browser/extension_message_filter.cc?rcl=4af5a8646e7a71978cf5e00dd62c4ab8926eaf10&l=158

So !render_prcess_host can definitely happen, I'll add the null check.

From the fuzzer name (linux_asan_chrome_ipc), it seems like it is fuzzing ipcs, so I don't really know how in practice we can reproduce this. That would make it impossible to write a test. Oh well.

/cc karandeepb@ for visibility (I'll send him the code review).
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 25 2017 

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

commit bd742967b7968f79667539b6c40c52082feba5ad
Author: Istiaque Ahmed <lazyboy@chromium.org>
Date: Tue Jul 25 04:38:48 2017

Add a null check while retrieving RPH in UIThreadWorkerResponseCallbackWrapper.

It seems BrowserMessageFilter can receive IPC after the render process
associated with it becomes inavlid. Since we depend on the RenderProcessHost
in UIThreadWorkerResponseCallbackWrapper, add a null check and bailout
in ExtensionFunctionDispatcher before creating a
UIThreadWorkerResponseCallbackWrapper instance.

No tests were added for the lack of reproducibility.

Bug:  747105 
Change-Id: I5838b7b7c35e1790dc843a4d4ff4f32a99312606
Reviewed-on: https://chromium-review.googlesource.com/581882
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489231}
[modify] https://crrev.com/bd742967b7968f79667539b6c40c52082feba5ad/extensions/browser/extension_function_dispatcher.cc

Comment 5 by lazyboy@chromium.org, Jul 27 2017

Status: Fixed (was: Started)
Though I couldn't repro this locally, marking hoping clusterfuzz will detect the fix...
►

Sign in to add a comment