Last Comment Bug 635705 - (CVE-2011-0078) Lack of NULL check in OOM allows arbitrary write
: Lack of NULL check in OOM allows arbitrary write
Product: Core
Classification: Components
Component: Networking (show other bugs)
: unspecified
: x86 All
: -- critical (vote)
: mozilla2.0
Assigned To: Jonathan Kew (:jfkthame)
Depends on:
  Show dependency treegraph
Reported: 2011-02-21 06:28 PST by Ian Beer
Modified: 2016-01-19 16:46 PST (History)
2 users (show)
rforbes: sec‑bounty+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch, check for OOM before writing the null terminator (877 bytes, patch)
2011-02-21 08:58 PST, Jonathan Kew (:jfkthame)
bzbarsky: review+
bzbarsky: approval2.0+
dveditz: approval1.9.2.17+
dveditz: approval1.9.1.19+
Details | Diff | Review
PoC with heapspray which can hit and fail the ::Clone call (1.05 KB, text/html)
2011-02-24 22:33 PST, Ian Beer
no flags Details

Description Ian Beer 2011-02-21 06:28:11 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-GB; rv: Gecko/20101206 Ubuntu/10.10 (maverick) Firefox/3.6.13
Build Identifier: 3.6.13

When parsing MIME types (for example the "text/javascript" here: <script type="text/javascript">...) the following code in netwerk/mime/src/nsMIMEHeaderParamImpl.cpp is called:

function nsMIMEHeaderParamImpl::GetParameterInternal

    158   // aParamName is empty. return the first (possibly) _unnamed_ 'parameter'
    159   // For instance, return 'inline' in the following case:
    160   // Content-Disposition: inline; filename=.....
    161   if (!aParamName || !*aParamName) 
    162     {
    163       for (; *str && *str != ';' && !nsCRT::IsAsciiSpace(*str); ++str)
    164         ;
    165       if (str == start)
    166         return NS_ERROR_UNEXPECTED;
    167       *aResult = (char *) nsMemory::Clone(start, (str - start) + 1);
    168       (*aResult)[str - start] = '\0';  // null-terminate
    169       NS_ENSURE_TRUE(*aResult, NS_ERROR_OUT_OF_MEMORY);
    170       return NS_OK;
    171     }

If the Clone call fails (returning NULL), then a null byte is still written to a controlled arbitrary address (0x0 + (str - start)) (where str-start is the length of the MIME type string.)

The address which could be written to does depend on the size of the string which is required to fail the Clone call, but it's still useably arbitrary (and, importantly, is an absolute address :-) ).

Lack of a mapped null page in modern OSes doesn't stop this, since 0x0 is never dereferenced here but a controlled, suitably large offset from it.

One possible exploitation scenario would be using this vuln to overwrite a refcount somewhere, setting it to 0 allowing a use-after-free.

I don't have a PoC unfortunately, I tried to knock up some suitable heap grooming JS but I couldn't get the right layout to trigger the Clone failing. That's not to say it's not possible!

The following html will hit the line of code though:

<script type="text/javascriptAAAAAAAAAAAA"> </script>

[obviously, you want a MUCH longer run of AAA's :-) ]

Reproducible: Couldn't Reproduce

Steps to Reproduce:
Exhaust memory suitably, document.write() a script tag with a long MIME type
Comment 1 Jonathan Kew (:jfkthame) 2011-02-21 08:58:39 PST
Created attachment 514026 [details] [diff] [review]
patch, check for OOM before writing the null terminator

I don't know how easy it would be to actually create an exploit for this, but the code certainly looks wrong. A simple fix should be to move the check for out-of-memory before the line that writes the null-terminator.
Comment 2 Boris Zbarsky [:bz] 2011-02-22 11:33:38 PST
Comment on attachment 514026 [details] [diff] [review]
patch, check for OOM before writing the null terminator

r+a=me.  Please land this!

And probably on branches too!
Comment 3 Jonathan Kew (:jfkthame) 2011-02-22 12:45:28 PST
Comment on attachment 514026 [details] [diff] [review]
patch, check for OOM before writing the null terminator

The same patch applies to 1.9.2 and 1.9.1 (just moved into the netwerk/mime/src directory); requesting approval to land on branches as well.
Comment 4 Jonathan Kew (:jfkthame) 2011-02-22 23:31:26 PST
Pushed to trunk:
Comment 5 Daniel Veditz [:dveditz] 2011-02-23 10:48:57 PST
Comment on attachment 514026 [details] [diff] [review]
patch, check for OOM before writing the null terminator

Approved for and, a=dveditz for release-drivers
Comment 6 Daniel Veditz [:dveditz] 2011-02-23 10:51:09 PST
There are locations where stomping a null byte can make all the difference in the world, but with things like ASLR it would seem pretty hard to make a reliable exploit out of this. Especially if you're running close to OOM. Is this really a plausible attack ('critical') or more theoretical ('moderate' or 'low')?
Comment 7 Ian Beer 2011-02-23 13:21:33 PST
I agree totally that without either a separate address leak bug or the target using a non-ALSR'ed platform reliable exploitation of this sort of bug is unlikely.

Windows XP is still both supported and widely used though :-(

(I've just rebooted an XP vm a couple of times and attached olly to firefox, the stack appears to be at a static base. I don't really know much about windows though, or whether stuff like EMET would help that?)

I'll offer another possible way to leverage this bug to achieve code execution:

Given the base address of the stack, a byte of a function pointer or return address on the stack can be set to 0x00. Successful exploitation would require finding a suitable pivot (or 'ROP gadget' if you want) at such an address such that the instruction sequence at that address was, for example, something like 'sub esp, 0xXX; ret', where 0xXX would move esp into a stack buffer you control. Coupled with a ret-to-lib DEP defeat this would yield arbitrary execution.

I would say that almost all 'critical' memory corruption bugs would still require either an address leak or to be on a non-aslr'ed platform to be turned into a reliable exploit (without a heap spray.)

I'd say this was a critical bug, but it's not my decision :-)
Comment 8 Ian Beer 2011-02-23 14:34:36 PST
(What is certainly true is that it would be a lot of effort to turn this from the theoretical attack outlined above to a plausible one)
Comment 9 Ian Beer 2011-02-23 14:43:35 PST
(I might have mis-interpreted your point, sorry, you're also quite right that even triggering this code (getting exactly the right ::Clone call to fail) would be pretty hard too!)
Comment 10 Ian Beer 2011-02-24 08:03:40 PST
BUG 443299 was a pretty similar bug for which a PoC exploit was written though :-)
Comment 11 Ian Beer 2011-02-24 22:31:13 PST
Inspired by BUG 443299 I played a bit more with my heapspraying js and I can now get the clone to fail in a repeatable way :-)

It's not an exploit yet by any means but it does at least show that you can hit this bug. In fact, because it exhausts almost all the address space, in all my tests it doesn't even crash... but watching in gdb you can see the null byte being written to the controlled address.

I'll paste my condensed gdb trace here and attach my PoC, I should point out though that it working first time for someone else in it's current state is unlikely! But I'm happy to work with you to fiddle with the magic numbers/phase of the moon etc to get a confirmation that it actually works for someone else!

This is on ubuntu 10.10:


(gdb) break nsMIMEHeaderParamImpl.cpp:168
Breakpoint 1 at 0xb73686a1: file nsMIMEHeaderParamImpl.cpp, line 168.
(gdb) c

Breakpoint 1, nsMIMEHeaderParamImpl::GetParameterInternal (this=0xb183d330, aHeaderValue=0xbfffe1e8 "text/javascript", 
    aParamName=0x0, aCharset=0xbfffdd5c, aLang=0x0, aResult=0xbfffdd54) at nsMIMEHeaderParamImpl.cpp:168
168	      (*aResult)[str - start] = '\0';  // null-terminate
(gdb) c

Breakpoint 1, nsMIMEHeaderParamImpl::GetParameterInternal (this=0xb183d330, 
    aHeaderValue=0xb8000008 "text/javascript", 'B' <repeats 185 times>..., aParamName=0x0, aCharset=0xbfffc980, aLang=0x0, 
    aResult=0xbfffc978) at nsMIMEHeaderParamImpl.cpp:168
168	      (*aResult)[str - start] = '\0';  // null-terminate
(gdb) print *aResult
$1 = 0x0
(gdb) info registers
eax            0x0	0
ecx            0xbfffc978	-1073755784
edx            0x0	0
ebx            0xb7f83fb0	-1208467536
esp            0xbfffc834	0xbfffc834
ebp            0xbfffc8ec	0xbfffc8ec
esi            0xb8000008	-1207959544
edi            0x600000f	100663311
eip            0xb73686a1	0xb73686a1 <nsMIMEHeaderParamImpl::GetParameterInternal(char const*, char const*, char**, char**, char**)+261>
eflags         0x282	[ SF IF ]
cs             0x73	115
ss             0x7b	123
ds             0x7b	123
es             0x7b	123
fs             0x0	0
gs             0x33	51
(gdb) disassemble
   0xb736868f <+243>:	lea    0x1(%edi),%eax
   0xb7368692 <+246>:	push   %eax
   0xb7368693 <+247>:	push   %esi
   0xb7368694 <+248>:	call   0xb7be05fa <nsMemory::Clone(void const*, PRSize)>
   0xb7368699 <+253>:	mov    -0x80(%ebp),%ecx
   0xb736869c <+256>:	add    $0x10,%esp
   0xb736869f <+259>:	mov    %eax,(%ecx)
=> 0xb73686a1 <+261>:	movb   $0x0,(%eax,%edi,1)
   0xb73686a5 <+265>:	cmpl   $0x0,(%ecx)
   0xb73686a8 <+268>:	jne    0xb7368bc1 <nsMIMEHeaderParamImpl::GetParameterInternal(char const*, char const*, char**, char**, char**)+1573>
   0xb73686ae <+274>:	jmp    0xb7368bba <nsMIMEHeaderParamImpl::GetParameterInternal(char const*, char const*, char**, char**, char**)+1566>
(gdb) info proc mapping
process 19367
cmdline = '/usr/lib/firefox-3.6.13/firefox-bin'
cwd = '/home/XXXXXXX/firefox-3.6.13+build3+nobinonly/mozilla/netwerk/mime/src'
exe = '/usr/lib/firefox-3.6.13/firefox-bin'
Mapped address spaces:

	Start Addr   End Addr       Size     Offset objfile
	  0x110000   0x11c000     0xc000          0    /usr/lib/firefox-3.6.13/firefox-bin
	  0x11c000   0x11d000     0x1000     0xb000    /usr/lib/firefox-3.6.13/firefox-bin
	  0x11d000   0x11e000     0x1000     0xc000    /usr/lib/firefox-3.6.13/firefox-bin
	  0x11e000   0x11f000     0x1000          0           [heap]
	 0x2500000  0xe600000  0xc100000          0        
	0x12500000 0x2a700000 0x18200000          0      
(gdb) x/4xb 0x600000f
0x600000f:	0x00	0x42	0x00	0x42
(gdb) set *(char *)(0x600000f) = 'F'
(gdb) x/4xb 0x600000f
0x600000f:	0x46	0x42	0x00	0x42
(gdb) n
(gdb) x/4xb 0x600000f
0x600000f:	0x00	0x42	0x00	0x42
(gdb) c


You can see that the target address (0x600000f) is mapped (and writable, but info proc mappings doesn't show that) so there isn't a segfault. In fact, at that address there's already a null byte (at least in the four or so times that I tried it) from the unicode version of the heapspray string "BBBBBBB". I wrote 'F' where it's about to write 0x00, stepped to the next line and you can see that a null byte was written there.
Comment 12 Ian Beer 2011-02-24 22:33:04 PST
Created attachment 515005 [details]
PoC with heapspray which can hit and fail the ::Clone call
Comment 13 Ian Beer 2011-02-24 22:34:20 PST
I also have a few scripts for monitoring the fragmentation of process address spaces which you'll almost certainly need to get this working at the moment. I'll attach them if needed/wanted

Note You need to log in before you can comment on or make changes to this bug.