Closed Bug 635705 (CVE-2011-0078) Opened 13 years ago Closed 13 years ago

Lack of NULL check in OOM allows arbitrary write

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla2.0

Tracking Flags:

Tracking

Status

blocking1.9.2

---

needed

status1.9.2

---

.17-fixed

blocking1.9.1

---

needed

status1.9.1

---

.19-fixed

People

(Reporter: ianpbeer, Assigned: jfkthame)

Details

(Whiteboard: [sg:critical?])

Attachments

(2 files)

patch, check for OOM before writing the null terminator 13 years ago Jonathan Kew [:jfkthame] 877 bytes, patch	bzbarsky : review+ bzbarsky : approval2.0+ dveditz : approval1.9.2.17+ dveditz : approval1.9.1.19+	Details \| Diff \| Splinter Review
PoC with heapspray which can hit and fail the ::Clone call 13 years ago Ian Beer 1.05 KB, text/html		Details

Ian Beer

Reporter

Description

•

13 years ago

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.2.13) 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:

<html><head>
<script type="text/javascriptAAAAAAAAAAAA"> </script>
</head><body>x</body></html>

[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

Jonathan Kew [:jfkthame]

Assignee

Comment 1

•

13 years ago

Attached patch patch, check for OOM before writing the null terminator — Details — Splinter Review

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.

Attachment #514026 - Flags: review?

Jonathan Kew [:jfkthame]

Assignee

Updated

•

13 years ago

Attachment #514026 - Flags: review? → review?(sdwilsh)

Boris Zbarsky [:bzbarsky]

Updated

•

13 years ago

Assignee: nobody → jfkthame

Status: UNCONFIRMED → NEW

Ever confirmed: true

Boris Zbarsky [:bzbarsky]

Comment 2

•

13 years ago

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!

Attachment #514026 - Flags: review?(sdwilsh)

Attachment #514026 - Flags: review+

Attachment #514026 - Flags: approval2.0+

Jonathan Kew [:jfkthame]

Assignee

Comment 3

•

13 years ago

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.

Attachment #514026 - Flags: approval1.9.2.15?

Attachment #514026 - Flags: approval1.9.1.18?

Jonathan Kew [:jfkthame]

Assignee

Comment 4

•

13 years ago

Pushed to trunk: http://hg.mozilla.org/mozilla-central/rev/f8279991d58f

Status: NEW → RESOLVED

Closed: 13 years ago

Resolution: --- → FIXED

Daniel Veditz [:dveditz]

Updated

•

13 years ago

blocking1.9.1: --- → ?

blocking1.9.2: --- → ?

status1.9.1: --- → wanted

status1.9.2: --- → wanted

Daniel Veditz [:dveditz]

Comment 5

•

13 years ago

Comment on attachment 514026 [details] [diff] [review]
patch, check for OOM before writing the null terminator

Approved for 1.9.2.15 and 1.9.1.18, a=dveditz for release-drivers

Attachment #514026 - Flags: approval1.9.2.15?

Attachment #514026 - Flags: approval1.9.2.15+

Attachment #514026 - Flags: approval1.9.1.18?

Attachment #514026 - Flags: approval1.9.1.18+

Daniel Veditz [:dveditz]

Comment 6

•

13 years ago

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')?

Whiteboard: [sg:moderate]

Daniel Veditz [:dveditz]

Updated

•

13 years ago

blocking1.9.1: ? → needed

blocking1.9.2: ? → needed

Ian Beer

Reporter

Comment 7

•

13 years ago

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 :-)

Ian Beer

Reporter

Comment 8

•

13 years ago

(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)

Ian Beer

Reporter

Comment 9

•

13 years ago

(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!)

Ian Beer

Reporter

Comment 10

•

13 years ago

BUG 443299 was a pretty similar bug for which a PoC exploit was written though :-)

Ian Beer

Reporter

Comment 11

•

13 years ago

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
Continuing.

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
Continuing.

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
169	      NS_ENSURE_TRUE(*aResult, NS_ERROR_OUT_OF_MEMORY);
(gdb) x/4xb 0x600000f
0x600000f:	0x00	0x42	0x00	0x42
(gdb) c
Continuing.

***********************************

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.

Ian Beer

Reporter

Comment 12

•

13 years ago

Attached file PoC with heapspray which can hit and fail the ::Clone call — Details

Ian Beer

Reporter

Comment 13

•

13 years ago

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

Jonathan Kew [:jfkthame]

Assignee

Comment 14

•

13 years ago

Pushed to 1.9.1 and 1.9.2 branches:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f51e2d406a0c
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/eaca9531b263

status1.9.1: wanted → .18-fixed

status1.9.2: wanted → .16-fixed

Daniel Veditz [:dveditz]

Updated

•

13 years ago

status1.9.2: .15-fixed → .16-fixed

Daniel Veditz [:dveditz]

Updated

•

13 years ago

Alias: CVE-2011-0078

Daniel Veditz [:dveditz]

Updated

•

13 years ago

Group: core-security

Target Milestone: --- → mozilla2.0

Daniel Veditz [:dveditz]

Updated

•

13 years ago

Whiteboard: [sg:moderate] → [sg:critical?]

Raymond Forbes[:rforbes]

Updated

•

11 years ago

Flags: sec-bounty+

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