1325200 - (CVE-2017-5375) lack of executable-code quota allows full bypass of ASLR and DEP

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Rh0]

Attachment: PoC for Firefox 50.1.0 (Release). Demonstrates point 2.1 in the report

asm.js allows to JIT-spray attacker controlled code, and hence, allows to fully
bypass ASLR and DEP in 32-bit (x86) Firefox (Tested versions: Release and
Nightly). As it is very easy to find, I do not know if it is already known (I
hope I do not reinvent the wheel here...)

In summary this means, that an attacker can exploit a memory corruption
vulnerability (such as an UAF) as if no ASLR and DEP existed. He only needs EIP
control and does not need any memory disclosures/info-leaks or code-reuse. That
makes exploitation of memory corruptions for an attacker super easy.


Consider following asm.js script:

<script>
function asm_js_module(){
    "use asm"
    function asm_js_function(){
        /* attacker controlled asm.js code */
    }
    return asm_js_function
}

modules = []
/* create 0x1000 executable regions containing our code */
for (i=0; i<0x1000; i++){
    /* request asm.js module */
    modules[i] = asm_js_module() 
}
</script>


Each time an asm.js module is requested, VirtualAlloc is called and returns a
new RW (PAGE_READWRITE) region aligned to 0xXXXX0000. At that point the call
stack looks like this (Firefox 50.1.0):

0:000> kP a
 # ChildEBP RetAddr  
00 008fe060 670ef66e KERNEL32!VirtualAllocStub
01 (Inline) -------- xul!js::jit::AllocateExecutableMemory+0x10 [c:\builds\moz2_slave\m-rel-w32-00000000000000000000\build\src\js\src\jit\executableallocatorwin.cpp @ 190]
02 008fe078 670f65c7 xul!AllocateCodeSegment(
            class js::ExclusiveContext * cx = 0x04516000, 
            unsigned int totalLength = <Value unavailable error>)+0x23 [c:\builds\moz2_slave\m-rel-w32-00000000000000000000\build\src\js\src\asmjs\wasmcode.cpp @ 67]
03 008fe0b8 670de070 xul!js::wasm::CodeSegment::create(
            struct JSContext * cx = 0x04516000, 
            class mozilla::Vector<unsigned char,0,js::SystemAllocPolicy> * bytecode = 0x08c61008, 
            struct js::wasm::LinkData * linkData = 0x08c61020, 
            struct js::wasm::Metadata * metadata = 0x06ab68d0, 
            class JS::Handle<js::WasmMemoryObject *> memory = class JS::Handle<js::WasmMemoryObject *>)+0x67 [c:\builds\moz2_slave\m-rel-w32-00000000000000000000\build\src\js\src\asmjs\wasmcode.cpp @ 206]
04 008fe184 6705f99d xul!js::wasm::Module::instantiate(
            struct JSContext * cx = 0x04516000, 
            class JS::Handle<JS::GCVector<JSFunction *,0,js::TempAllocPolicy> > funcImports = class JS::Handle<JS::GCVector<JSFunction *,0,js::TempAllocPolicy> >, 
            class JS::Handle<js::WasmTableObject *> tableImport = class JS::Handle<js::WasmTableObject *>, 
            class JS::Handle<js::WasmMemoryObject *> memoryImport = class JS::Handle<js::WasmMemoryObject *>, 
            class mozilla::Vector<js::wasm::Val,0,js::SystemAllocPolicy> * globalImports = 0x008fe200, 
            class JS::Handle<JSObject *> instanceProto = class JS::Handle<JSObject *>, 
            class JS::MutableHandle<js::WasmInstanceObject *> instanceObj = class JS::MutableHandle<js::WasmInstanceObject *>)+0x94 [c:\builds\moz2_slave\m-rel-w32-00000000000000000000\build\src\js\src\asmjs\wasmmodule.cpp @ 689]
05 008fe260 6705aae6 xul!TryInstantiate(
            struct JSContext * cx = 0x04516000, 
            class JS::CallArgs args = class JS::CallArgs, 
            class js::wasm::Module * module = 0x08c61000, 
            struct js::AsmJSMetadata * metadata = 0x06ab68d0, 
            class JS::MutableHandle<js::WasmInstanceObject *> instanceObj = class JS::MutableHandle<js::WasmInstanceObject *>, 
            class JS::MutableHandle<JSObject *> exportObj = class JS::MutableHandle<JSObject *>)+0x1e6 [c:\builds\moz2_slave\m-rel-w32-00000000000000000000\build\src\js\src\asmjs\asmjs.cpp @ 7894]
06 008fe2c4 35713638 xul!InstantiateAsmJS(
            struct JSContext * cx = 0x04516000, 
            unsigned int argc = 0, 
            class JS::Value * vp = 0x008fe2f0)+0x88 [c:\builds\moz2_slave\m-rel-w32-00000000000000000000\build\src\js\src\asmjs\asmjs.cpp @ 8008]


Afterwards, the asm.js compiled/native code is copied to the RW region
(firefox-50.1.0/js/src/asmjs/WasmCode.cpp:223 in method CodeSegment::create):

223         memcpy(cs->code(), bytecode.begin(), bytecode.length());
224         StaticallyLink(*cs, linkData, cx);
225         if (memory)
226             SpecializeToMemory(*cs, metadata, memory);
227     }
228 
229     if (!ExecutableAllocator::makeExecutable(cs->code(), cs->codeLength())) {

And in line 229 the RW region is made executable (PAGE_EXECUTE_READ) with
ExecutableAllocator::makeExecutable which calls VirtualProtect.


An attacker can request one asm.js module many times, which leads to the
creation of many RX regions. Due to the allocation granularity of VirtualAlloc
he can then choose a fixed address (such as 0x1c1c0000) and can be certain that
his compiled/native asm.js code is located there.


IMO following flaws/missing checks are responsible that asm.js JIT-spray is
possible. Some of them are well known:

1) Not MANY, but only ONE RX region should be created per module. Many requests
to one asm.js module should return references instead of new RX regions.


2.1) Constant folding is missing in Release (50.1.0). (But it seems it was
added in Nightly between 18 Nov. 2016 and 30 Nov. 2016). This makes it possible
to use many subsequent arithmetic operations on one variable and wrap the
legitimate instructions.
        
For example, we can hide three NOPS (0x90) in asm.js constants with:
    VAL = (VAL + 0xA8909090)|0;
    VAL = (VAL + 0xA8909090)|0;

Following native code is generated:
    00: 05909090A8    ADD EAX, 0xA8909090
    05: 05909090A8    ADD EAX, 0xA8909090

When we jump into offset 01 (the immediate of the first instruction) we can
execute our hidden code (We'll use 0xA8 as masking byte):
    01: 90    NOP
    02: 90    NOP
    03: 90    NOP
    04: A805  TEST AL, 05
    06: 90    NOP
    07: 90    NOP
    08: 90    NOP
    09: A8...


2.2) Although constant folding/propagation seems to be implemented in Nightly,
an attacker can still force to emit his hidden code: for example, he can hide
two nops and a jump to the next constant with following asm.js code:
    asm_js_heap[0x10] = 0xEB069090;
    asm_js_heap[0x11] = 0xEB069090;

asm.js emits following native code:
    00: B89090EB06	MOV EAX, 0x06EB9090
    05: A340C07B04	MOV DWORD [0x047BC040], EAX
    0A: B89090EB06	MOV EAX, 0x06EB9090
    0F: A344C07B04	MOV DWORD [0x047BC044], EAX

But when we jump into offset 01, we get:
    01: 90    NOP
    02: 90    NOP
    03: EB06  JMP 0x0B
    ..
    0B: 90    NOP
    0C: 90    NOP
    0D: EB06  JMP 0x15
    ..


2.3) Another possibility is to use FFI and pass many parameters to it.
Consider following asm.js code:
    heap[10] = ffi(0x90909090|0, 0x90909090|0, 0x90909090|0, 0x90909090|0, /* and many more*/ )

Following native code is emitted (Nightly 53.0a1 20161215061212 32-bit - Windows 10):
    0:026> u 1c1c0053
    1c1c0053 90              nop
    1c1c0054 90              nop
    1c1c0055 90              nop
    1c1c0056 c744240c90909090 mov     dword ptr [esp+0Ch],90909090h
    1c1c005e c744241090909090 mov     dword ptr [esp+10h],90909090h
    1c1c0066 c744241490909090 mov     dword ptr [esp+14h],90909090h
    1c1c006e c744241890909090 mov     dword ptr [esp+18h],90909090h
    1c1c0076 c744241c90909090 mov     dword ptr [esp+1Ch],90909090h

This again allows to use two bytes as payload bytes and two bytes as jump to
the next constant.


3) There is no constant blinding of 4-byte constants. Hence, an attacker can
use the well known trick to hide his opcodes/payload/shellcode within the
constants (as shown in 2.1, 2.2, 2.3).


4) There is insufficient random nop insertion. It seems that the same amount
and type of nops are emitted for a specific module. That means, that the native
code of one module is fully predictable. An attacker can test his code and use
it across other machines with the same OS/Firefox version (am I missing here
sth.?). Better random nop insertion would break 2.1, 2.2, 2.3. More random
base addresses, each time the same module is used would help, too.  Separating
code from data such as using a constant pool for constants might also help.


To demonstrate that this asm.js JIT-spray works, I attached several proof of
concepts for Windows 8.1 and Windows 10 (including an exploit for
CVE-2016-9079, the recent Tor Browser 0day). I have also exploits for
CVE-2016-1960 and CVE-2016-2819, but the attached PoCs should proof the point.
Information how to test them is available in the PoCs.

Principally, asm.js JIT-spray should work for Linux, too. However, mmap returns
regions on a 4 KB granularity, hence more modules need to be sprayed to occupy
a predictable address. I did not investigate any possibilities for 64-bit
Firefox. However it might also be affected in one or another way.

I think JIT hardening was also tracked by
https://bugzilla.mozilla.org/show_bug.cgi?id=677272.

Please correct me if anything is wrong or if I missed something. Or ping me if
one of the PoCs does not work or if you need more information.

More general information about JIT-spray can be found here:
https://packetstormsecurity.com/files/86975/Writing-JIT-Spray-Shellcode.pdf
http://zhodiac.hispahack.com/my-stuff/security/Flash_Jit_InfoLeak_Gadgets.pdf
https://www.nccgroup.trust/globalassets/resources/us/presentations/documents/attacking_clientside_jit_compilers.pdf
https://www.nccgroup.trust/globalassets/resources/us/presentations/documents/attacking_clientside_jit_compilers_paper.pdf
https://media.blackhat.com/bh-us-11/Tsai/BH_US_11_TsaiPan_Weapons_Targeted_Attack_Slides.pdf
https://erpscan.com/wp-content/uploads/2012/06/HITB-JIT-Spray-Attacks-and-Advanced-Shellcode.pdf
http://xlab.tencent.com/en/2015/12/09/bypass-dep-and-cfg-using-jit-compiler-in-chakra-engine/
https://sites.google.com/site/bingsunsec/WARPJIT/JIT%20Spraying%20Never%20Dies%20-%20Bypass%20CFG%20By%20Leveraging%20WARP%20Shader%20JIT%20Spraying.pdf



Best,

    Rh0

Comment 1

[Rh0]

Attachment: Another PoC for Firefox 50.1.0 (Release) to demonstrate point 2.1

Comment 2

[Rh0]

Attachment: PoC for Nightly. Demonstrates point 2.2

Comment 3

[Rh0]

Attachment: PoC Exploit for CVE-2016-9079 (Firefox 50.0.1) using asm.js JIT-Spray to bypass ASLR and DEP. No info-leak and code-reuse is utilized.

Comment 4

[Daniel Veditz [:dveditz]]

Hannes: who should look at this one?

Comment 5

[Luke Wagner [:luke]]

Do we have any JIT-spray prevention for normal JIT code or is it just the fact that JIT code compilation is more complicated and thus harder to predict?

If we offset the entire module's code by some random offset (maintaining 16-byte alignment) within the entire RW allocation, would that be sufficient to prevent placing code at predictable addresses?

Comment 6

[Jan de Mooij [:jandem]]

For non-asm.js/wasm code, we use ExecutableAllocator::computeRandomAllocationAddress on Windows and pass the result of that to VirtualAlloc. Should we do something similar for asm.js/wasm? Not sure it would help here, I didn't look closely at the POC.

It might also be worth limiting how much executable memory a Zone/JSRuntime can allocate, I vaguely remember Chakra doing this. We do have MaxWasmCodeAllocations FWIW.

Comment 7

[Rh0]

(In reply to Jan de Mooij [:jandem] from comment #6)
> For non-asm.js/wasm code, we use
> ExecutableAllocator::computeRandomAllocationAddress on Windows and pass the
> result of that to VirtualAlloc. Should we do something similar for
> asm.js/wasm? Not sure it would help here, I didn't look closely at the POC.
> 

I quickly looked at ExecutableAllocator::computeRandomAllocationAddress (50.1.0 source, 32-bit) and think that this would not really help, because VirtuaAlloc would still deliver 64-KB aligned chunks. Although the base addresses become more random, I suspect that enough requests to one asm.js module would yield predictable addresses at some point (This is a bit of a wild guess right now, I didn't do the exact calculations yet)

Comment 8

[Rh0]

(In reply to Luke Wagner [:luke] from comment #5)

> If we offset the entire module's code by some random offset (maintaining
> 16-byte alignment) within the entire RW allocation, would that be sufficient
> to prevent placing code at predictable addresses?

If the offset is different within each RW region that would help against point 2.2 and 2.3. But it would still be possible (with a chance of 4/5) to hit the nop sled and land in attacker controlled code (point 2.1).

Comment 9

[Luke Wagner [:luke]]

I expect the same nop-sled attack would work against normal JIT code since we're using the same backend.  So that's a separate mitigation we should consider.

Rather, it seems like the problem here is lack of quota allowing the attacker to fill up the address space with executable code (which defeats randomization).  For asm.js/wasm, though, MaxWasmCodeAllocations isn't quite good enough since it's just number of modules and one can easily generate 100mb modules.  But if we added a parallel MaxWasmCodeBytes, I think that would work and not be too hard.

Does normal JIT code have a quota?

Comment 10

[Jan de Mooij [:jandem]]

(In reply to Luke Wagner [:luke] from comment #9)
> Does normal JIT code have a quota?

No, we should probably fix that at the same time...

Comment 11

[Hannes Verschore [:h4writer]]

Is there something actionable we can do here?

Comment 12

[Jan de Mooij [:jandem]]

Luke, what about the following:

(1) Rejigger things so we use computeRandomAllocationAddress for asm.js/wasm code allocations as well.

(2) Add |Atomic<uint32_t> codeBytesAllocated;| so we can enforce how much executable memory a single process can allocate. I think this is nicer than having it on the runtime, because a malicious website could create many workers to spray the heap. I'm just not sure what a reasonable quota would be for the whole process...

Comment 13

[Jan de Mooij [:jandem]]

Also, how feasible would it be to use the ExecutableAllocator (maybe a different instance) for wasm executable memory? That could also be more efficient when there are many small modules, just one RX mapping.

Comment 14

[Luke Wagner [:luke]]

Jan: yes, (1) and (2) sound like exactly what I was thinking as well.  Would you be able to add codeBytesAllocated and the ExecutableAllocator integration?  After that, (1) and (2) are trivial to do in WasmCode.cpp.

Regarding ExecutableAllocator: that's a good question.  We used to mprotect() all the code (for the interrupt handler) which required page alignment, but not anymore.  The other reason we (still) need page alignment of the code pages is that they need to be followed by some number of r+w global data pages.  When we (soon) implement code-sharing-between-instances, we'll be moving all the writable global data out, leaving only some read-only process-invariant data (pointers to C functions, etc).  Then we could make the entire allocation r+x and thus page-alignment-agnostic and switch to ExecutableAllocator.

Comment 15

[Jan de Mooij [:jandem]]

I'll try something here.

Comment 16

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Randomize asm.js/wasm code addresses on Windows

This patch renames ExecutableAllocator::computeRandomAllocationAddress to ComputeRandomAllocationAddress and calls it from AllocateExecutableMemory instead of ExecutableAllocator::systemAlloc, so it works for *all* code allocations.

This also lets us remove the |addr| argument from AllocateExecutableMemory.

One issue was that we used a XorShift128PlusRNG in ExecutableAllocator for this, but we don't have a good place to store this state for ComputeRandomAllocationAddress. I removed this RNG and now we just call js::GenerateRandomSeed on each allocation. This looks fine perf-wise, is simpler, and as bonus should also give us a stronger RNG for this.

Pretty happy with how this turned out.

Comment 17

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Limit executable memory per process

This puts a hard per-process limit on the executable memory we allocate. On 32-bit platforms the limit is 128 MB, on 64-bit platforms 512 MB (because the virtual address space is much larger there).

JS_ShutDown asserts the counter is 0 to make sure the counter is updated correctly.

The asserts here caught a pre-existing OOM bug actually (we called systemRelease twice in createPool). This should be extremely rare in non-OOM-testing scenarios and hard to exploit, but it's a nice find anyway :)

Comment 18

[Jan de Mooij [:jandem]]

I forgot to mention: part 2 does not remove the wasmCodeAllocations mechanism we no longer need. We can do that as a follow-up though.

Comment 19

[Luke Wagner [:luke]]

Comment on attachment 8825903 [details] [diff] [review]
Part 1 - Randomize asm.js/wasm code addresses on Windows

Review of attachment 8825903 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

Comment 20

[Jan de Mooij [:jandem]]

Note that there's still the problem of VirtualAlloc having a granularity of 64K :( That will be fixed when we use the ExecutableAllocator for wasm code, as discussed, but until then I wonder if we should also cap the *number* of allocations to account for this.

Maybe we should lower MaxWasmCodeAllocations to 1000 or so on 32-bit, to stop the "spray many small modules" attack?

Comment 21

[Luke Wagner [:luke]]

Comment on attachment 8825908 [details] [diff] [review]
Part 2 - Limit executable memory per process

Review of attachment 8825908 [details] [diff] [review]:
-----------------------------------------------------------------

Great!

::: js/src/jit/ExecutableAllocator.h
@@ +339,5 @@
>  
>  extern void
>  DeallocateExecutableMemory(void* addr, size_t bytes, size_t pageSize);
>  
> +extern MOZ_MUST_USE bool

Maybe add comment "These functions are called by the platform-specific definitions of (Allocate|Deallocate)ExecutableMemory and should not otherwise be called directly."?

Comment 22

[Luke Wagner [:luke]]

(In reply to Jan de Mooij [:jandem] from comment #20)
Hmm, maybe I'm missing something but I don't see how it's a problem that wasm modules are 64k aligned given that their overall address is (with these patches) randomized and the max % of address space occupied by code quite limited.

Comment 23

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Jan, we will gtb the last Beta of this cycle in the next 12-24 hrs. Do you think we can get this fix ready to land asap? Otherwise we will need to uplift this in RC1 which seems a bit risky.

Comment 24

[Jan de Mooij [:jandem]]

(In reply to Luke Wagner [:luke] from comment #22)
> (In reply to Jan de Mooij [:jandem] from comment #20)
> Hmm, maybe I'm missing something but I don't see how it's a problem that
> wasm modules are 64k aligned given that their overall address is (with these
> patches) randomized and the max % of address space occupied by code quite
> limited.

When you allocate 2^16 modules (64 KB alignment each), the whole address space is filled up on 32-bit, so there are 65536 possibilities. Let's say each module is 2 pages. A 128 MB limit lets you then allocate 16384 modules: 128 MB / 8192 KB. That means a JIT spray still has a decent chance of success (1 out of 4 or so).

Comment 25

[Luke Wagner [:luke]]

(In reply to Jan de Mooij [:jandem] from comment #24)
Oh, I see, I was taking the probability to be 1 in 32 (4gb / 128mb) but I was indeed forgetting about the hidden VirtualAlloc unit size.  Good point!  Really it sounds like, on Windows, we need to round up the amount we increment allocatedExecutableBytes by to 64kb (regardless of wasm, because that is the underlying effect on overall address space randomization).

Comment 26

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Limit executable memory per process v2

Like the previous version, but rounds up to 64 KB on Windows.

Comment 27

[Jan de Mooij [:jandem]]

Comment on attachment 8825903 [details] [diff] [review]
Part 1 - Randomize asm.js/wasm code addresses on Windows

(Applies to both patches.)

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
It can be used to bypass ASLR/DEP once an attacker has an exploit for another bug.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No, or at least not more than is obvious from the code.

> Which older supported branches are affected by this flaw?
All.

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Should apply or be easy to backport.

> How likely is this patch to cause regressions; how much testing does it need?
We will now limit the amount of executable memory to 128 MB per process on 32-bit (and 512 MB on 64-bit). The biggest risk is users getting OOMs from this. I don't expect it, as 128 MB is a lot of code and we purge JIT code quite aggressively, but it is somewhat of a risk.

Comment 28

[Jan de Mooij [:jandem]]

(In reply to Ritu Kothari (:ritu) from comment #23)
> Hi Jan, we will gtb the last Beta of this cycle in the next 12-24 hrs. Do
> you think we can get this fix ready to land asap? Otherwise we will need to
> uplift this in RC1 which seems a bit risky.

If I get my approvals shortly, and no surprises show up on Try, I will make sure this is on all branches within 12 hours.

Comment 29

[Jan de Mooij [:jandem]]

Comment on attachment 8825903 [details] [diff] [review]
Part 1 - Randomize asm.js/wasm code addresses on Windows

(Applies to both patches.)

Approval Request Comment
[Feature/Bug causing the regression]: Old issue.
[User impact if declined]: Security issues.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Some risk.
[Why is the change risky/not risky?]: We now have a limit on the amount of executable memory we allow per process. I don't think users are likely to hit this in practice, but it's possible.
[String changes made/needed]: None.

Comment 30

[Jan de Mooij [:jandem]]

I'll land this on inbound when I see some green on Try.

Comment 31

[Daniel Veditz [:dveditz]]

Comment on attachment 8825903 [details] [diff] [review]
Part 1 - Randomize asm.js/wasm code addresses on Windows

This can certainly land on Aurora. We so close to the wire on the RC1 build that the release managers should approve this for beta and ESR so they aren't surprised.

Comment 32

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab1fd5b320a457e8313dfcdb2d90c99850f3f1d9
https://hg.mozilla.org/integration/mozilla-inbound/rev/21dab2e6926ae09b8a63dc7653dcf4e7c6f4efa9

Comment 33

[Jan de Mooij [:jandem]]

Attachment: Part 3 - Discard regexp/constructor code

I just realized we don't discard RegExp and unboxed-constructor code, so we may hold onto largely-unused 64 KB pools in that case and reach our code limit faster. Here's a small patch to fix that; we now discard these JitCodes on compacting GC.

Comment 34

[Jan de Mooij [:jandem]]

Attachment: Patch for Aurora

Comment 35

[Jan de Mooij [:jandem]]

Attachment: Patch for Beta

Comment 36

[Jan de Mooij [:jandem]]

Attachment: Patch for ESR45

I left out the RegExp-code discarding part of part 3, it doesn't apply cleanly and is less important for the ESR branch. This also removes the bogus RandomizeIsBroken code that bug 1232907 removed in 46; it's less risk to use the same code everywhere.

Comment 37

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8826222 [details] [diff] [review]
Patch for Aurora

Please land this for aurora (rather than patches 1 and 2 - this replaces them)

Comment 38

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8825903 [details] [diff] [review]
Part 1 - Randomize asm.js/wasm code addresses on Windows

Fixing the flags on patches 1 and 2 - there are specific branch patches instead.

Comment 39

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8826225 [details] [diff] [review]
Patch for Beta

Please uplift to beta.

Comment 40

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8826242 [details] [diff] [review]
Patch for ESR45

Rebased for ESR45.

Comment 41

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d51ff865f7da04783da5431d287ed24d57ee5a0e
https://hg.mozilla.org/releases/mozilla-beta/rev/24aa31d493ed8cdfcf77d4d39693ec8f83ef29d7

Will push the ESR patch in a bit.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr45/rev/ead08c2a6c57

Comment 43

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ab1fd5b320a4
https://hg.mozilla.org/mozilla-central/rev/21dab2e6926a
https://hg.mozilla.org/mozilla-central/rev/165b23f87d08
https://hg.mozilla.org/mozilla-central/rev/ee94a9d6cc1c

Comment 44

[Gerry Chang [:gchang]]

Track 51+ as sec-critical.

Comment 45

[Luke Wagner [:luke]]

(Renaming bug to reflect the generality of the final fix.)

Comment 46

[Daniel Veditz [:dveditz]]

The fix that was actually made did not address several of the problems pointed out in the initial comment. Are there bugs raised on the other issues? The fundamental problem here is that the attacker can create valid machine code in executable memory. This patch makes it harder for an attack to get to it, but it's still sitting there waiting to be found.

Comment 47

[Jan de Mooij [:jandem]]

(In reply to Daniel Veditz [:dveditz] from comment #46)
> The fix that was actually made did not address several of the problems
> pointed out in the initial comment. Are there bugs raised on the other
> issues? The fundamental problem here is that the attacker can create valid
> machine code in executable memory. This patch makes it harder for an attack
> to get to it, but it's still sitting there waiting to be found.

I filed bug 1330780 on inserting a random number of bytes between functions, but as discussed there we'd first need some mitigation against nop slides, maybe some form of constant blinding as suggested here. Unfortunately constant blinding is hard to do in a way that's both really effective and has acceptable performance overhead. For instance, comment 0 mentions blinding 4 byte constants, but ISTR some papers suggest you need to do this even for 2-byte constants to be effective. Last I checked, no other engines take the performance hit from this (except Chakra maybe) or they do it only in very limited cases like their Baseline JIT.

I'll file some followup bugs though to investigate our options.

Comment 48

[Bogdan Maris, Desktop Test Engineering]

Hi, can you please offer some guidance if this needs manual QA? I saw in comment 29 that no QA is required but the bug has qe-verify+.

Things that I did and saw until now:
I am running the poc files in 50.0.1 and I got a notification "Set EIP to 0x1c1c0053 (Firefox Release <= 50.1.0 32-bit - Windows 8.1/10) with your vuln here or in WinDBG" and in 51.0 I don't (for any of the poc files). I see that I need to set EIP to different values, after a quick search I saw that i need to use OllyDbg, but don't know how to use that.

Comment 49

[Matt Wobensmith [:mwobensmith][:matt:]]

Hi Bogdan, we had hoped the verification would just involve running the PoC files to confirm the change in behavior. You've done that, so maybe this bug should be considered verified.

Regarding setting EIP, OllyDbg, etc. - maybe Jan can offer some guidance.

Comment 50

[Matt Wobensmith [:mwobensmith][:matt:]]

Hi Rh0, can you confirm that this has been fixed correctly? Thank you.

Comment 51

[Jan de Mooij [:jandem]]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #48)
> I am running the poc files in 50.0.1 and I got a notification "Set EIP to
> 0x1c1c0053 (Firefox Release <= 50.1.0 32-bit - Windows 8.1/10) with your
> vuln here or in WinDBG" and in 51.0 I don't (for any of the poc files).

It's expected you don't see the message in Firefox 51. We now OOM because we allocate too much JIT code so we don't get to the alert. That confirms the quota fix works correctly.

Comment 52

[Rh0]

(In reply to Matt Wobensmith [:mwobensmith][:matt:] from comment #50)
> Hi Rh0, can you confirm that this has been fixed correctly? Thank you.

The above attached PoCs are mitigated but I think I have found a way to circumvent the fixes. The basic idea is that the same tricks can be used as in heap feng shui / heap spray (Allocate memory, deallocate memory) to create allocation holes at predictable base address. These holes can then be used to place jit-sprayed regions there.

Simple Example (very first test):

1) Allocate many unrelated memory regions (e.g.: Uint32Array(0x100000)) to occupy very many 64 KB base addresses. This reserves (but does not commit) memory up to an target address (e.g.: 0x50000000)

2) Jit-spray max allowed executable memory. Last time i checked MaxCodeBytesPerProcess was at 160*1024*1024 bytes (source: Firefox 51.0).  The smallest asm-js/JIT module chunk is 64*1024, so an attacker can spray up to (190*1024*1024)/(64*1024) = 2560 modules to occupy 64KB-aligned base addresses with controlled code.

3) Then, I think due to few memory space available, 2560 modules seems enough: the code of fix-part1 will fallback to "normal" VirtualAlloc instead of using randomAddr for VirtualAlloc.

4) Memory allocated in 1) may be released by triggering the garbage collector.

In (very) first tests I was able to get attacker controlled code reliably to predictable addresses in version 51.0 (win32) and Nightly (from Jan. 23 2017; win32).

I'll provide PoC later on. (Should I file a new bug on it?)

Thanks!
Rh0

Comment 53

[Ryan VanderMeulen [:RyanVM]]

(In reply to Rh0 from comment #52)
> I'll provide PoC later on. (Should I file a new bug on it?)

Yes, please. It'll make tracking the new issue much easier. And thank *you* for the report!

Comment 54

[Rh0]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #53)
> (In reply to Rh0 from comment #52)
> > I'll provide PoC later on. (Should I file a new bug on it?)
> 
> Yes, please. It'll make tracking the new issue much easier. And thank *you*
> for the report!

I opened bug 1334933 to demonstrate the issue.

Comment 55

[Bogdan Maris, Desktop Test Engineering]

(In reply to Matt Wobensmith [:mwobensmith][:matt:] from comment #49)
> Hi Bogdan, we had hoped the verification would just involve running the PoC
> files to confirm the change in behavior. You've done that, so maybe this bug
> should be considered verified.

Then I'll go ahead and mark this bug as Verified Fixed since I also verified it now on Firefox 52beta 3, latest Developer Edition 53.0a2 and Firefox 45.7.0 ESR on Windows 8.1/10 both 32bit.

Comment 56

[Frederik Braun [:freddy]]

The reporter blogged about this bug and the exploit at https://rh0dev.github.io/blog/2017/the-return-of-the-jit/.

Dan, can we make this bug public?