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

Lack of NULL check in OOM allows arbitrary write

Categories

(Core :: Networking, defect)

Product:
Core
Component:
Networking
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

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
(deleted), 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
(deleted), text/html
Details
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
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?
Attachment #514026 - Flags: review? → review?(sdwilsh)
Assignee: nobody → jfkthame
Status: UNCONFIRMED → NEW
Ever confirmed: true
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+
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?
Pushed to trunk: http://hg.mozilla.org/mozilla-central/rev/f8279991d58f
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
status1.9.1: --- → wanted
status1.9.2: --- → wanted
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+
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]
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
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 :-)
(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)
(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!)
BUG 443299 was a pretty similar bug for which a PoC exploit was written though :-)
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.
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
Alias: CVE-2011-0078
Attachment #528176 - Attachment description: Bug Bounty Nomination → Bug Bounty Awarded $3000
Group: core-security
Target Milestone: --- → mozilla2.0
Whiteboard: [sg:moderate] → [sg:critical?]
Attachment #528176 - Attachment description: Bug Bounty Awarded $3000 → Bug Bounty Awarded $3000 - to accounting aug 12
Attachment #528176 - Attachment description: Bug Bounty Awarded $3000 - to accounting aug 12 → Bug Bounty Awarded $3000 - [paid]
Flags: sec-bounty+
Attachment #528176 - Attachment description: Bug Bounty Awarded $3000 - [paid] → ianpbeer@gmail.com ,3000,2011-02-21,2011-02-22,2011-04-25,true
Attachment #528176 - Attachment filename: bugbountynom.txt → bugbounty.data
Attachment #528176 - Attachment description: ianpbeer@gmail.com ,3000,2011-02-21,2011-02-22,2011-04-25,true → ianpbeer@gmail.com ,3000,2011-02-21,2011-05-20,2011-04-25,true
Attachment #528176 - Attachment description: ianpbeer@gmail.com ,3000,2011-02-21,2011-05-20,2011-04-25,true → ianpbeer@gmail.com ,3000,2011-02-21,2011-05-20,2011-04-25,true,,,
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: