Issue metadata
Sign in to add a comment
|
CRASH() writes to a fixed mappable address
Reported by
jannhorn@googlemail.com,
Sep 20 2016
|
||||||||||||||||||||||
Issue description
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.113 Safari/537.36
Steps to reproduce the problem:
1. On Linux, trigger a CRASH() somehow.
2. Look at 'dmesg | grep segfault'.
What is the expected behavior?
Chrome should terminate safely.
What went wrong?
In dmesg:
[87568.538035] chrome[23293]: segfault at fbadbeef ip 000055e98e040812 sp 00007ffcc2e99bb0 error 6 in chrome[55e98a195000+631d000]
This is triggered by CRASH():
#ifndef CRASH
#if COMPILER(MSVC)
#define CRASH() (__debugbreak(), IMMEDIATE_CRASH())
#else
#define CRASH() \
(WTFReportBacktrace(), (*(int*)0xfbadbeef = 0), IMMEDIATE_CRASH())
#endif
#endif
CRASH() first calls WTFReportBacktrace(), then attempts to kill the process by writing to 0xfbadbeef, and if that wasn't enough, kills the process through __builtin_trap() via IMMEDIATE_CRASH().
There are two issues with this:
- With 32-bit Chromium on a 64-bit kernel (and with x32, which I think Chromium doesn't support
yet, and theoretically also in a normal 64-bit binary on a 64-bit kernel), 0xfbadbeef can be
a valid userland address. (On a 64-bit Linux kernel, the limit for allocations by 32-bit binaries is
0xFFFFe000 unless the ADDR_LIMIT_3GB personality flag is active, see IA32_PAGE_OFFSET in the
kernel sources.)
- The access is a write, not just a read.
This means that four bytes of random memory might be zeroed before the process exits. If there are other threads, this could theoretically permit an attacker to exploit this as a memory corruption on another thread, especially when you keep in mind that the operating system might need some time to stop all threads after a crash.
Did this work before? N/A
Chrome version: 53.0.2785.113 Channel: stable
OS Version:
Flash Version: Shockwave Flash 23.0 r0
I think that it would make sense to change `(*(int*)0xfbadbeef = 0)` to `(*(volatile int*)((sizeof(int*)==8) ? 0xFFFFFFFFFFFFFBAD : 0xFFFFFBAD))`. This way, the crash address doesn't depend on whether something happens to be mapped at 0xfbadbeef, and even if something can somehow be mapped at the magic address, at least it won't be overwritten. `volatile` prevents the compiler from compiling out the dereference.
This issue is probably not exploitable on many machines because:
- 32-bit Chromium on a 64-bit kernel is probably a relatively uncommon
configuration, at least on Linux. (Although maybe it happens if 32-bit
Android apps on 64-bit devices use webviews or so?)
- On 64-bit Linux, I don't think anything is allocated below 0x550000000000 or
so, at least unless all the address space above that address is filled up
first.
,
Sep 20 2016
We no longer ship 32-bit linux or mac builds. (We do ship 32-bit android on 64-bit kernels.) jannhorn: what do you think about (WTFReportBacktrace(), (*(int*)0x2000 = 0), IMMEDIATE_CRASH()) The first page should be read-only everywhere these days. But going just to IMMEDIATE_CRASH() is probably even better.
,
Sep 20 2016
It would be helpful (not necessary) if we kept something like this when ASan is enabled. We started relying on it in CF for certain cases, but maybe we just shouldn't. I do agree that just going to IMMEDIATE_CRASH in production builds is better.
,
Sep 20 2016
I agree - if it doesn't break things, IMMEDIATE_CRASH() is probably the best option. In case that doesn't work, I agree with thakis that a write to 0x2000 should also be fine. > We no longer ship 32-bit linux or mac builds. Debian does ship a 32-bit x86 chromium build though: https://packages.debian.org/jessie/chromium
,
Sep 22 2016
,
Sep 22 2016
,
Sep 22 2016
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cf2b885f31494c8006875e56cddef2840fffc9fa commit cf2b885f31494c8006875e56cddef2840fffc9fa Author: thakis <thakis@chromium.org> Date: Thu Sep 22 16:57:39 2016 Don't write to an arbitrary address before crashing in blink's CRASH macro We could write to the first page which is write-protected everywhere, but let's see if we can just remove this. (If this breaks anyone's workflow, please let me know.) BUG= 648620 Review-Url: https://codereview.chromium.org/2361843002 Cr-Commit-Position: refs/heads/master@{#420374} [modify] https://crrev.com/cf2b885f31494c8006875e56cddef2840fffc9fa/third_party/WebKit/Source/wtf/Assertions.h
,
Sep 23 2016
,
Dec 30 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 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by elawrence@chromium.org
, Sep 20 2016Labels: Security_Severity-Low Security_Impact-Stable
Owner: thakis@chromium.org
Status: Assigned (was: Unconfirmed)