Closed Bug 1182002 Opened 9 years ago Closed 8 years ago

Mark freed memory with something more likely to mitigate exploits than 0x5a

Categories

(Core :: Memory Allocator, defect)

Product:
Core
Component:
Memory Allocator
Version:
39 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox42 --- fixed
firefox-esr38 42+ fixed

People

(Reporter: abbGZcvu_bugzilla.mozilla.org, Unassigned)

References

Details

(Keywords: sec-high, Whiteboard: [poc in bug 1179484 comment 10]) 

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150630154324

Steps to reproduce:

I wrote a PoC for bug 1179484, where freed memory is filled with 0x5a before being reused.


Actual results:

The PoC successfully controls the instruction pointer by spraying the address 0x5a5a5a5a


Expected results:

The freed memory should have been marked with a value that is less likely to result in addresses that are within reach of a heap spray. Good candidates are values in the range 0xC0-0xFF.
There are quite a few places where this happens:
https://dxr.mozilla.org/mozilla-central/search?q=regexp%3A\bmemset\s*\%28.*\b0x5a\b&case=false
An open question is how the poison value should change based on OS memory map characteristics (I presume the suggested range is for Windows).
Status: UNCONFIRMED → NEW
Component: Untriaged → Memory Allocator
Ever confirmed: true
Product: Firefox → Core
Yes, I was indeed referring to Windows. x86 Windows will only allow userland allocations up to address 0x7FFF,FFFF (or 0xBFFF,FFFF when using /3G, see http://blogs.technet.com/b/askperf/archive/2007/03/23/memory-management-demystifying-3gb.aspx). 

I currently know of these markers in various applications and libraries on Windows: [citation needed]

  0xABABABAB Uninitialized malloc memory
  0xBBADBEEF Safari reads/writes to this address to kill on assertion failure
  0xBAADF00D Uninitialized LMEM_FIXED memory
  0xCCCCCCCC Uninitialized local variable
  0xC0C0C0C0 Uninitialized heap memory
  0xCDCDCDCD Uninitialized LocalAlloc memory
  0xD0D0D0D0 Uninitialized heap memory
  0xDDDDDDDD Freed memory
  0xF0F0F0F0 Freed memory
  0xFDFDFDFD Unused canary memory
  0xF0DE7FFF Poisoned nsFrame
  0xF0090100 Poisoned nsFrame
  0xFEEEFEEE Freed memory

There do not appear to be any markers in use that start with 0xE0-0xEF, so I would suggest using values in that range.

Memory gets filled in multiple location in the code and not just after free. It would be nice to use different values for each of them. That way the crash address may reveal some details about the type of memory allocation involved and the cause (use-after-free vs uninitialized).

I am not familiar with the memory layout on x64 Windows, so I cannot advise if these values would be reasonable there as well.

I have not written an exploit for *nix in a long time, so I cannot advise on reasonable values there.
Summary: Mark freed memory with something more likely to mitigate use-after-free than 0x5a → Mark freed memory with something more likely to mitigate exploits than 0x5a
See Also: → 1044077
> There do not appear to be any markers in use that start with 0xE0-0xEF, so I
> would suggest using values in that range.
> 
> Memory gets filled in multiple location in the code and not just after free.
> It would be nice to use different values for each of them. That way the
> crash address may reveal some details about the type of memory allocation
> involved and the cause (use-after-free vs uninitialized).

Yeah. There is a similar recommendation in bug 1044077 comment 4 and 5.
Can I have access to that bug?
I think this is effectively a dupe of bug 1108693, though it is good to know that this is a real problem and not just a theoretical one.
njn, can you find somebody to fix this? (And ideally bug 1044077.)
Flags: needinfo?(n.nethercote)
Whiteboard: [poc in bug 1179484 comment 10]
Let me see if I understand this correctly.

We want a canary value that has two properties:

- The value will not lead to addressable memory when used as a pointer (this bug and bug 1108693).

- The value will not lead to a valid instruction sequence (bug 1044077).

0x5a provides neither of those properties.

I suspect 0x00 would meet both those properties, but I imagine there's also a third property:

- The value is "obvious" in a crash report.

Is that right?

For Unix platforms, is it safe to assume they are 64-bit? I imagine there are some large chunks of the 64-bit address space that are off-limits because real hardware only uses 48-bit pointers, AIUI. In contrast, I don't think any part of a 32-bit Linux address space is guaranteed to be unaddressable except for the very first page.
Well, you can always _make_ address space inaccessible, by mapping that region yourself. Trouble is, something might already be there.
(0x00 has the unfortunate property that it would cause null-checks to fail and possibly paper over a bunch of problems, or put us into weird state for later. And as an instruction, 00 00 comes out to "add byte ptr [eax], al" which doesn't always crash)
(In reply to David Major [:dmajor] from comment #9)
> Well, you can always _make_ address space inaccessible, by mapping that
> region yourself. Trouble is, something might already be there.

Which is why the poison value for mozilla/Poison.h is variable. But it's trickier to do that for the memory allocator.
(In reply to Mike Hommey [:glandium] from comment #11)
> (In reply to David Major [:dmajor] from comment #9)
> > Well, you can always _make_ address space inaccessible, by mapping that
> > region yourself. Trouble is, something might already be there.
> 
> Which is why the poison value for mozilla/Poison.h is variable. But it's
> trickier to do that for the memory allocator.

(except if it does it itself, that is)
froydnj pointed at mfbt/Poison.h which gives us a word that definitely satisfies the first property in comment 8, partially satisfies the second due to it being variable (for whatever that's worth), and doesn't satisfy the third. It's currently used for nsPresArena (which has a custom allocator) and nsView (not sure why it's used there).
We do annotate crash reports with the poison value (at least I think we do), so it's at least discoverable what's going on, if not immediately obvious by looking at the registers.
As a random point of information, or some weird reason the poison value shows up as null on OSX crash reports.
(In reply to Andrew McCreight [:mccr8] from comment #15)
> As a random point of information, or some weird reason the poison value
> shows up as null on OSX crash reports.

On Win64 crash reports, the 0x5a poison value gets listed as 0xffffffff`ffffffff since it's not a canonical 48-bit address.
Flags: needinfo?(n.nethercote)
Something like 0xe5 would be reasonable. Pros and cons:

+ It's not a nop-slide (see bug 1044077 comment 4).

+ It won't clash with any special marker memory regions on Windows (see comment 3).

+ It's obvious, and similar to the existing 0x5a value.

- Like any statically-chosen value, it could lead to addressable memory.

The other option is a dynamically-chosen poison value like the one from mfbt/Poison.h:

? It might be a nop-slide, or might not.

+ It probably(?) won't clash with special marker memory regions on Windows.

? It's not obvious, though it is mentioned in crash reports.

+ It won't lead to addressable memory.

- It's trickier to use in jemalloc (see comment 11).

How do we decide? I'm no expert on this stuff, and I have little sense of whether solving some but not all of the problems is useful or not.

Also are there any other options? A dynamically-chosen poison value with certain constraints might satisfy more criteria, but it would also be harder obtaining such a value.

What do other browsers do?
I vote for changing to 0xe5 as soon as feasible since it's better than 0x5a, and file a followup to investigate more-complex solutions that might satisfy more criteria.
Keywords: sec-high
> Something like 0xe5 would be reasonable. Pros and cons:
> 
> - Like any statically-chosen value, it could lead to addressable memory.

glandium explained to me that this is not necessarily true.

- On 32-bit Firefox running on 32-bit Windows, 0xe5e5e5e5 is above the 2 GiB user-space limit and so is not a valid address.

- On 64-bit Firefox running on 64-bit OSes (Windows, Mac, Linux), 0xe5e5e5e5`e5e5e5e5 is above the 2^48 physical pointer limit, and so it also not a valid address.

- On 32-bit Firefox running on 64-bit Windows user-space is 4 GiB, so 0xe5e5e5e5 might be a valid address. It's unclear if on

So we'd get protection for many configurations, which is better than I first thought.

So I'll try to write a patch to change it to 0xe5.
32-bit Firefox on 64-bit Windows is nearly half of our users, though. It would be good to add some code that reserves the address space around 0xe5e5e5e5 very early during startup. Maybe that can be a followup.
I submitted a patch to bug 1044077, since 0xe5 will solve that bug completely, while it will only solve this bug partially.
The fix in bug 1044077 was recently landed. What's left to do specifically for this bug?
Group: core-security → core-security-release
(In reply to Daniel Veditz [:dveditz] from comment #23)
> The fix in bug 1044077 was recently landed. What's left to do specifically
> for this bug?

Nothing, as far as I know. Though there's a lot of good information in this bug's comments about why 0xe4 and 0xe5 are good canary values. Putting that information into a comment in the code would be nice, but I don't know if it's wise to have a big explanation of the security-related strengths and weaknesses of a particular canary value front and centre.
Can we close this per comment 24?
Flags: needinfo?(n.nethercote)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #25)
> Can we close this per comment 24?

I think so.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(n.nethercote)
Resolution: --- → FIXED
The actual fixed release was 41/ESR-38.3 but those values are no longer settable for status tracking. 42/42+ is the closest I can get.
status-firefox42: --- → fixed
status-firefox-esr38: --- → fixed
tracking-firefox-esr38: --- → 42+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.