Sandbox escape due to miss check in TracingHostMsg_TriggerBackgroundTrace IPC message
Reported by
zhouat2...@gmail.com,
Nov 24 2017
|
||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.75 Safari/537.36 Steps to reproduce the problem: 1. route the normal other ipc message (render to browser ) to TracingHostMsg_TriggerBackgroundTrace handler. 2. see the SEGV_MAPERR 3. the ipc message handler is here: https://cs.chromium.org/chromium/src/content/browser/tracing/trace_message_filter.cc?l=43 What is the expected behavior? the Chromium process Received signal 11 SEGV_MAPERR due to null pointer access . i can reproduce both at chromium 63 and chromium 64. What went wrong? 1. missing check the config_ at source code: https://cs.chromium.org/chromium/src/content/browser/tracing/background_tracing_manager_impl.cc?type=cs&q=OnHistogramTrigger&sq=package:chromium&l=326 Did this work before? N/A Chrome version: 64.0.3277.0 Channel: n/a OS Version: Ubuntu 16.04.3 LTS \n \l Flash Version: Disabled
,
Nov 25 2017
Thank you for providing more feedback. Adding requester "jialiul@chromium.org" to the cc list and removing "Needs-Feedback" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 25 2017
zhouat2017, could you explain how this can be used in a (non-local) attack? If so, could you provide a better trigger? e.g. a crafted web-page etc
,
Nov 25 2017
I suspect the precondition of this attack is that you already have compromised the render process via a different RCE vulnerability in the Renderer process. You then use the issue described in this bug to attempt to compromise the Browser process.
,
Nov 27 2017
awhalley@, re#6, how do we usually treat this kind of reporting?
,
Nov 27 2017
hi - is it possible for you to upload a renderer patch that would call the IPC and cause the issue to happen in the browser? This would make reproducing this easier for us. How are you reproducing this yourself?
,
Nov 28 2017
oysteine: Looks like you own the relevant code. Could you please take a look? Thanks!
,
Nov 28 2017
,
Nov 28 2017
,
Nov 28 2017
,
Nov 29 2017
oysteine@ knows more about this code and looks like he is already on it. I have doubts on the severity of this. I think this is a bug and should be fixed, but IMO not even worth merging back more than beta. I don't think this is a sandbox escape though. I took a look at the code at it seems to me that what's going on is the following: if you manage to forge an TracingHostMsg_TriggerBackgroundTrace IPC to the browser too early (or I think even without the finch experiment running, where the config_ might be never set), the browser will dereference an empty unique_ptr and crash. I am not a security expert, but causing the browser to crash because of hitting a nullptr, doesn't look like a sandbox escape to me. I mean, you are escaping into sure death, because all chrome processes will then die.
,
Nov 29 2017
RE #15: Denial-of-service via NULL dereference was my analysis of the issue as well; we do not typically treat such issues as security issues (https://chromium.googlesource.com/chromium/src/+/master/docs/security/faq.md#Are-denial-of-service-issues-considered-security-bugs). The fact that it brings down the browser process makes it a more severe bug, but the fact that it requires a renderer compromise as a precondition in turn lowers the severity.
,
Nov 30 2017
root cause: (please refer to the decompiled code below for more detail info ) 1. chrome receive SIGSEGV when visit `.text:0000000002C5122A mov rbx, [rax+10h]`, <--- rax(config_) is 0x0 now, here cause the issue, which try to access [rax+0x10] (rules_). 2. rax come from `.text:0000000002C51226 mov rax, [r14+10h]`, <--- rax come form [r14+0x10h], which is rax(config_),now it is 0x0 . 3. r14 come from `.text:0000000002C51214 mov r14, rdi`, <--- rdi is `this pointer`, and saved to r14(this pointer) r14(this pointer) content is below, you see the pointer is not null, just the offset from [r14+0x10] is 0x0, which is only init at one place(here it try to access uninitialized memory, this can lead to Serious security issues). link address: https://cs.chromium.org/chromium/src/content/browser/tracing/background_tracing_manager_impl.cc?gsn=config_&l=180 gef⤠x/16gx $r14 0x55555d509310 <_ZN7content12_GLOBAL__N_128g_background_tracing_managerE+8>: 0x000055555cf76190 0x000004dc8bd2c6c0 0x55555d509320 <_ZN7content12_GLOBAL__N_128g_background_tracing_managerE+24>: 0x0000000000000000 0x000055555d509330 0x55555d509330 <_ZN7content12_GLOBAL__N_128g_background_tracing_managerE+40>: 0x0000000000000000 0x0000000000000000 0x55555d509340 <_ZN7content12_GLOBAL__N_128g_background_tracing_managerE+56>: 0x0000000000000000 0x0000000000000000 0x55555d509350 <_ZN7content12_GLOBAL__N_128g_background_tracing_managerE+72>: 0x0000000000000000 0x0000000000010000 0x55555d509360 <_ZN7content12_GLOBAL__N_128g_background_tracing_managerE+88>: 0x00000000ffffffff 0x000004dc8bd2d8a0 0x55555d509370 <_ZN7content12_GLOBAL__N_128g_background_tracing_managerE+104>: 0x000004dc8bd2d8a0 0x0000000000000001 0x55555d509380 <_ZN7content12_GLOBAL__N_128g_background_tracing_managerE+120>: 0x000004dc8c42ec60 0x000004dc8c42ec60 /* decompiled code */ .text:0000000002C51200 ; content::BackgroundTracingManagerImpl::OnHistogramTrigger(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) .text:0000000002C51200 _ZN7content28BackgroundTracingManagerImpl18OnHistogramTriggerERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEE proc near .text:0000000002C51200 ; CODE XREF: content::TraceMessageFilter::OnTriggerBackgroundTrace(std::__1::basic_string<char,std::__1::char_traits<char>,std::__1::allocator<char>> const&)+1Aj .text:0000000002C51200 ; DATA XREF: content::BackgroundTracingManagerImpl::OnHistogramTrigger(std::__1::basic_string<char,std::__1::char_traits<char>,std::__1::allocator<char>> const&)+C4o .text:0000000002C51200 .text:0000000002C51200 var_60 = byte ptr -60h .text:0000000002C51200 var_40 = byte ptr -40h .text:0000000002C51200 var_38 = byte ptr -38h .text:0000000002C51200 var_30 = qword ptr -30h .text:0000000002C51200 .text:0000000002C51200 push rbp .text:0000000002C51201 mov rbp, rsp .text:0000000002C51204 push r15 .text:0000000002C51206 push r14 .text:0000000002C51208 push r13 .text:0000000002C5120A push r12 .text:0000000002C5120C push rbx .text:0000000002C5120D sub rsp, 38h .text:0000000002C51211 mov r12, rsi .text:0000000002C51214 mov r14, rdi <--- .text:0000000002C51217 xor edi, edi .text:0000000002C51219 call _ZN7content13BrowserThread11CurrentlyOnENS0_2IDE ; content::BrowserThread::CurrentlyOn(content::BrowserThread::ID) .text:0000000002C5121E test al, al .text:0000000002C51220 jz short loc_2C51282 .text:0000000002C51222 mov [rbp+var_30], r14 .text:0000000002C51226 mov rax, [r14+10h] <--- .text:0000000002C5122A mov rbx, [rax+10h] <--- .text:0000000002C5122E mov r14, [rax+18h] The code at https://cs.chromium.org/chromium/src/content/browser/tracing/background_tracing_manager_impl.cc?type=cs&q=OnHistogramTrigger&sq=package:chromium&l=326 will try to visit uninitialized memory, this lead to Serious security issues. To fix this problem, you can refer to the patch i offer here , should add check at two place. /* patch file start */ --- /home/zhouat/chrome_linux_2017_11_24/src/src-64.0.3276.2/content/browser/tracing/background_tracing_manager_impl.cc 2017-11-23 19:09:46.000000000 -0800 +++ background_tracing_manager_impl.cc 2017-11-23 20:09:50.238165886 -0800 @@ -322,6 +322,9 @@ base::Unretained(this), histogram_name)); return; } + + if(!config_ || config_->rules().empty()) + return; for (const auto& rule : config_->rules()) { if (rule->ShouldTriggerNamedEvent(histogram_name)) /* patch file end */ Thanks.
,
Dec 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07fad9439e5b1ab4b8f458963c553601c1e16ff1 commit 07fad9439e5b1ab4b8f458963c553601c1e16ff1 Author: Oystein Eftevaag <oysteine@chromium.org> Date: Thu Dec 07 18:59:01 2017 Background tracing: Ensure config is present on histogram trigger IPCs R=ssid@chromium.org Bug: 788306 Change-Id: I2e81ab0c1836aebeb2ed36e02c38e39309557057 Reviewed-on: https://chromium-review.googlesource.com/793897 Reviewed-by: Siddhartha S <ssid@chromium.org> Commit-Queue: oysteine <oysteine@chromium.org> Cr-Commit-Position: refs/heads/master@{#522492} [modify] https://crrev.com/07fad9439e5b1ab4b8f458963c553601c1e16ff1/content/browser/tracing/background_tracing_manager_impl.cc
,
Dec 7 2017
,
Dec 8 2017
,
Dec 11 2017
,
Dec 14 2017
The VRP panel took a look at his, and concluded that this is an unexploitable null pointer dereference, sorry!
,
Dec 18 2017
> is there some specification can ensure that, the _config must be null? Yes: |config_| is a std::unique_ptr, not a raw pointer. As per [1] the default empty constructor for std::unique_ptr zero-initializes the pointer. > even though the _config must be null, some times null pointer dereference can lead to privilege escalation, you can refer here for info That requires that you are already able to execute arbitrary code to perform a mmap(MAP_FIXED) @ address 0. But if you can execute arbitrary code to do that mmap you won already, without needing to exploit a null pointer dereference. [1] http://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr
,
Jan 2 2018
Hello - I'm afraid since this isn't being classed as a security bug, it won't be getting a CVE, sorry.
,
Mar 16 2018
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 |
||||||||||||||
Comment 1 by jialiul@chromium.org
, Nov 25 2017