A previous post analyzed a stack buffer-overflow in the parse_master function of VLC <=v0.9.4. parse_master is susceptible to another vulnerability, this time of the heap-overflow variety.
* Following this analysis requires some understanding of Intel assembly and basic reverse engineering concepts.
Explanation of Source Code
When VLC plays a .ty or any other file and encounters a certain sequence of bytes [13] it calls the parse_master function with p_demux as an argument [15]. p_demux is the remaining (unprocessed) bytes of the input video file. The peek function [14] compares the current 4 bytes in the input stream to the magic bytes [13] and then rewinds p_demux to the offset beginning with those same bytes (in other words, if the magic bytes are encountered, parse_master is passed the input stream (p_demux) starting with the offset of the magic bytes).
#define TIVO_PES_FILEID ( 0xf5467abd ) [13]
...
/* check if it's a PART Header */
if( U32_AT( &p_peek[ 0 ] ) == TIVO_PES_FILEID ) [14]
{
/* parse master chunk */
parse_master(p_demux); [15]
As you can see in the image below, the byte stream begins with the magic bytes “F5 46 7a bd”.
The parse_master function begins with declaring a series of variables including an array of 32 8-bit integers (mst_buf) [1] as well as two 32-bit integers (i and i_map_size) [2]. Further down, there is a call to thhe stream_Read function which reads 32 bytes from the input stream into mst_buf [3]. The following line [4], sets i_map_size to the 32-bit value located at mst_buf[20]. The variable i is then initialized [6] to the 32-bit value at the end of the mst_buf buffer (mst_buf[28]). Finally, the i_seq_table_size data element in the p_sys structure is set to the result of the expression i / (8 + i_map_size) [7].
demux_sys_t *p_sys = p_demux->p_sys;
uint8_t mst_buf[32]; [1]
int i, i_map_size; [2]
...
stream_Read(p_demux->s, mst_buf, 32); [3]
i_map_size = U32_AT(&mst_buf[20]); /* size of bitmask, in bytes */[4]
p_sys->i_bits_per_seq_entry = i_map_size * 8;
i = U32_AT(&mst_buf[28]); /* size of SEQ table, in bytes */ [6]
p_sys->i_seq_table_size = i / (8 + i_map_size); [7]
After these initial variables are initialized, a malloc call is made [8] with the size argument passed as i_seq_table_size * sizeof(ty_seq_table_t) (the size of the ty_seq_table_t data element is 16). The resulting memory pointer is stored in seq_table.
p_sys->seq_table = malloc(p_sys->i_seq_table_size * sizeof(ty_seq_table_t)); [8]
Further down in the function, a memcpy call is made which takes i_map_size bytes from the mst_buf buffer and writes to the memory location pointed to by seq_table (the pointer returned from our previous malloc call [12]).
memcpy(p_sys->seq_table[i].chunk_bitmask, &mst_buf[8], i_map_size); [12]
Source Code Vulnerability Analysis
With an understanding of the source code, let’s analyze the vulnerability… Given i_map_size is a signed integer [1], if we set it to a negative number, say -1 (or FFFFFFFFh) [4] and set i to a value of 7 [6], we can get an i_seq_table_size equal to 1 [7]. Keep in mind, we can set the values of i_map_size and i arbitrarily [4] [6] since these values are parsed directly out of mst_buf which comes from user-supplied input. Now, when malloc is called [8] the size will be 16 (i_seq_table_size which is 1 multiplied by _sizeof(ty_seq_table_t) which is 16) which will return a pointer to a memory region with at-least 16 bytes of memory. Of note here is this is a relatively SMALL memory region (one that would be easier to overflow).
uint8_t mst_buf[32]; [1]
...
i_map_size = U32_AT(&mst_buf[20]); /* size of bitmask, in bytes */ [4]
p_sys->i_bits_per_seq_entry = i_map_size * 8;
i = U32_AT(&mst_buf[28]); /* size of SEQ table, in bytes */ [6]
p_sys->i_seq_table_size = i / (8 + i_map_size); [7]
/* parse all the entries */
p_sys->seq_table = malloc(p_sys->i_seq_table_size * sizeof(ty_seq_table_t)); [8]
Within the for loop declaration [9], we see it should execute i_seq_table_size amount of times which we know is 1 (so it should only iterate once through). Unlike the stack-overflow condition seen in a previous post, the stream_Read call within the for loop should execute with no issues (no overflow condition) as it is merely writing 7 bytes from the input stream into mst_buf [10]. In order to bypass the if condition [11] (which must be done to get to the memcpy function), i_map_size must be <= 8, which we know it is as we had previously set it to -1 (FFFFFFFFh). Finally, we get to the memcpy call [12] which writes FFFFFFFFh bytes from mst_buf into the memory location pointed to by seq_table. Since memcpy uses the FFFFFFFFh size value as an unsigned value, this is a very large amount of data it attempts to write into memory which overflows the allocated memory buffer which was only 16 when first passed to the malloc function earlier. This will result in an access violation and the program crashes due to overflowing the heap buffer!
for (i=0; i<p_sys->i_seq_table_size; i++) { [9]
stream_Read(p_demux->s, mst_buf, 8 + i_map_size); [10]
p_sys->seq_table[i].l_timestamp = U64_AT(&mst_buf[0]);
if (i_map_size > 8) { [11]
msg_Err(p_demux, "Unsupported SEQ bitmap size in master chunk");
memset(p_sys->seq_table[i].chunk_bitmask, i_map_size, 0);
} else {
memcpy(p_sys->seq_table[i].chunk_bitmask, &mst_buf[8], i_map_size); [12]
Assembly Code Vulnerability Analysis
Analysis of the vulnerability continues by inspecting the disassembled code…
Initializing i_map_size variable from the mst_buf buffer
After the first of the two stream_Read calls (0x61401C1F), the FFFFFFFFh value passed in via the user inputted .ty file is loaded (via instructions 0x61401C24-0x61401C62) onto the stack and stored at offset 0x0629FB04 (ESP+A0).
61401C24 0FB6B424 D400000>MOVZX ESI,BYTE PTR SS:[ESP+D4]
61401C2C 0FB69C24 D500000>MOVZX EBX,BYTE PTR SS:[ESP+D5]
61401C34 0FB68C24 D700000>MOVZX ECX,BYTE PTR SS:[ESP+D7]
61401C3C 0FB69424 D600000>MOVZX EDX,BYTE PTR SS:[ESP+D6] ;these 4 grab FFFFFFFF from mst_buf stored on stack
61401C44 C1E6 18 SHL ESI,18
61401C47 89B424 A0000000 MOV DWORD PTR SS:[ESP+A0],ESI
61401C4E C1E3 10 SHL EBX,10
61401C51 099C24 A0000000 OR DWORD PTR SS:[ESP+A0],EBX
61401C58 C1E2 08 SHL EDX,8
61401C5B 098C24 A0000000 OR DWORD PTR SS:[ESP+A0],ECX ;these 6 instructions convert endianness of bytes from input buffer
61401C62 099424 A0000000 OR DWORD PTR SS:[ESP+A0],EDX ;storing in 060BFB04
Initializing i variable from the mst_buf buffer
The i variable has the user inputted value loaded into it from the mst_buf array. This value is stored on the stack at ESP+A4
61401C83 0FB68424 DC00000>MOVZX EAX,BYTE PTR SS:[ESP+DC]
61401C8B 0FB6B424 DD00000>MOVZX ESI,BYTE PTR SS:[ESP+DD]
61401C93 0FB69C24 DF00000>MOVZX EBX,BYTE PTR SS:[ESP+DF]
61401C9B 0FB68C24 DE00000>MOVZX ECX,BYTE PTR SS:[ESP+DE] ;grabs DWORD from input buffer
61401CA3 C1E0 18 SHL EAX,18
61401CA6 C1E6 10 SHL ESI,10
61401CA9 09F0 OR EAX,ESI
61401CAB C1E1 08 SHL ECX,8
61401CAE 09D8 OR EAX,EBX
61401CB0 09C8 OR EAX,ECX
61401CB2 89BC24 A4000000 MOV DWORD PTR SS:[ESP+A4],EDI ;store i value on stack
i_seq_table_size expression
At instruction 0x61401C70 the i_map_size value is loaded into EDI from the stack. 8 is added to it and then it is stored at an address on the stack.
61401C70 8BBC24 A0000000 MOV EDI,DWORD PTR SS:[ESP+A0]
…
61401C80 83C7 08 ADD EDI,8
…
61401CB2 89BC24 A4000000 MOV DWORD PTR SS:[ESP+A4],EDI
The stack now looks like…
0629FB04 FFFFFFFF ÿÿÿÿ
0629FB08 00000007 …
From here, the rest of i_seq_table_size is calculated.
61401CB9 99 CDQ ; sign extend EAX into EDX
61401CBA F7BC24 A4000000 IDIV DWORD PTR SS:[ESP+A4]
61401CC1 8985 C8BE0000 MOV DWORD PTR SS:[EBP+BEC8],EAX
malloc call
EAX, now with a value of 1 is shifted left to get a value of 10h (which is 16) and malloc is called with this size value. (Remember, malloc was called with a value of 16 as this is the size of ty_seq_table_t.)
61401CBA F7BC24 A4000000 IDIV DWORD PTR SS:[ESP+A4] ; SETS EAX to 1
…
61401CC7 C1E0 04 SHL EAX,4
61401CCA 890424 MOV DWORD PTR SS:[ESP],EAX
61401CCD E8 4E580000 CALL <JMP.&msvcrt.malloc>
malloc returns with a memory pointer in EAX of (in my case it is 0x04909420.)
No stream_Read overwrite issue
The following assembly instructions set up the second stream_Read call which has a size value parameter of 7 which will not overwrite the mst_buf size of 32.
61401D4A 894C24 04 MOV DWORD PTR SS:[ESP+4],ECX ;pointer to mst_buf
61401D4E 897424 08 MOV DWORD PTR SS:[ESP+8],ESI ;size value of 7
…
61401D57 893C24 MOV DWORD PTR SS:[ESP],EDI ;pointer to P_demux stream
61401D5A E8 31510000 CALL <JMP.&libvlccore.stream_Read>
Bypass if statement
The following shows the if statement in assembly ensuring that i_map_size is not greater than 8.
61401EA5 83BC24 A0000000 >CMP DWORD PTR SS:[ESP+A0],8
61401EAD 894C3B 04 MOV DWORD PTR DS:[EBX+EDI+4],ECX
61401EB1 ^0F8F 3AFEFFFF JG libty_pl.61401CF1 ; This jump is not taken as it is not greater
memcpy call
The following instructions set up and execute the memcpy.
61401EBF 8B9424 A0000000 MOV EDX,DWORD PTR SS:[ESP+A0] ; FFFFFFFF into EDX
61401EC6 01F1 ADD ECX,ESI ; ECX is 0 so this sticks ESI (malloc pointer) into ECX
61401EC8 83FA 07 CMP EDX,7 ; Compares EDX FFFFFFFF to 7
61401ECB 8D79 08 LEA EDI,DWORD PTR DS:[ECX+8] ;
61401ECE 8DB424 C8000000 LEA ESI,DWORD PTR SS:[ESP+C8] ;LEA on ESI which has malloc result pointer
61401ED5 76 29 JBE SHORT libty_pl.61401F00 ; continues on as last CMP set to 1
61401ED7 F7C7 04000000 TEST EDI,4 ; EDI is not 4 (its a memory pointer)
61401EDD 74 21 JE SHORT libty_pl.61401F00 ; this jump happens
Which jumps to…
61401F00 89D1 MOV ECX,EDX ; moves FFFFFFFF into ECX
…
61401F09 F3:A5 REP MOVS DWORD PTR ES:[EDI],DWORD PTR DS> ; memcpy
Here is a look at the registers at the time of the crash.
Registers at time of crash
EAX 00004A00
ECX 3FFF16CA
EDX FFFFFFFF
EBX 0487AEE8
ESP 061BFA64
EBP 047F7CE0
ESI 061FA000
EDI 048B53C4
EIP 61401F09 libty_pl.61401F09
The access violation occurs during the course of the memcpy call (specifically during the REP MOVS instruction). The violation references the memory address stored in ESI. This is evidence of the heap overflow!
61401F09 F3:A5 REP MOVS DWORD PTR ES:[EDI],DWORD PTR DS> ; memcpy
Patching the Code
The issue with the heap overflow described above is that you can have a memcpy that attempts to copy data of size i_map_size (which can be arbitrarily set and very large) into a small buffer. The only validation of i_map_size is done via the if condition [11] which checks to see if it is greater than 8. What this doesn’t consider is whether i_map_size is some value 0 or smaller (even up to FFFFFFFFh!). Implementing more robust validation of i_map_size to ensure it can only be a value between 1 and 8 is one way to mitigate the vulnerability.
So if we changed …
if (i_map_size > 8) {
to…
if (i_map_size < 1 || i_map_size > 8) {
then this could solve the issue!
Source Code
static void parse_master(demux_t *p_demux)
{
demux_sys_t *p_sys = p_demux->p_sys;
uint8_t mst_buf[32]; [1]
int i, i_map_size; [2]
int64_t i_save_pos = stream_Tell(p_demux->s);
int64_t i_pts_secs;
/* Note that the entries in the SEQ table in the stream may have
different sizes depending on the bits per entry. We store them
all in the same size structure, so we have to parse them out one
by one. If we had a dynamic structure, we could simply read the
entire table directly from the stream into memory in place. */
/* clear the SEQ table */
free(p_sys->seq_table);
/* parse header info */
stream_Read(p_demux->s, mst_buf, 32); [3]
i_map_size = U32_AT(&mst_buf[20]); /* size of bitmask, in bytes */ [4]
p_sys->i_bits_per_seq_entry = i_map_size * 8;
i = U32_AT(&mst_buf[28]); /* size of SEQ table, in bytes */ [6]
p_sys->i_seq_table_size = i / (8 + i_map_size); [7]
/* parse all the entries */
p_sys->seq_table = malloc(p_sys->i_seq_table_size * sizeof(ty_seq_table_t)); [8]
for (i=0; i<p_sys->i_seq_table_size; i++) { [9]
stream_Read(p_demux->s, mst_buf, 8 + i_map_size); [10]
p_sys->seq_table[i].l_timestamp = U64_AT(&mst_buf[0]);
if (i_map_size > 8) { [11]
msg_Err(p_demux, "Unsupported SEQ bitmap size in master chunk");
memset(p_sys->seq_table[i].chunk_bitmask, i_map_size, 0);
} else {
memcpy(p_sys->seq_table[i].chunk_bitmask, &mst_buf[8], i_map_size); [12]
}
}
================= CODE BREAK =====================
#define TIVO_PES_FILEID ( 0xf5467abd ) [13]
================= CODE BREAK =====================
/* check if it's a PART Header */
if( U32_AT( &p_peek[ 0 ] ) == TIVO_PES_FILEID ) [14]
{
/* parse master chunk */
parse_master(p_demux); [15]
return get_chunk_header(p_demux);
}