A few days after having published the post about the Bitdefender stack buffer overflow via 7z PPMD , I discovered a new bug in Bitdefender’s product. While this is a 7z bug, too, it has nothing to do with the previous bug or with the PPMD codec. Instead, it concerns dynamic memory management. In contrast to the previous post , which described an arbitrary free vulnerability in F-Secure’s anti-virus product, this post presents the first heap buffer overflow of this blog series.

Introduction

For the write-up on the 7z PPMD bug, I read a lot of the original 7-Zip source code and discovered a few new things that looked promising to investigate in anti-virus products. Therefore, I took another stab at analyzing Bitdefender’s 7z module.

I previously wrote about relaxed file processing. The Bitdefender 7z PPMD stack buffer overflow was a good example of relaxed file processing by removing a check (that is, removing code).

This bug demonstrates another fundamental difficulty that arises when incorporating new code into an existing code base. In particular, a minimal set of changes to the new code is often inevitable. Mostly, this affects memory allocation and code that is concerned with file access, especially if a totally different file abstraction is used. The presented bug is an example of the former type of difficulty. More specifically, an incorrect use of a memory allocation function that extends the 7-Zip source code in Bitdefender’s 7z module causes a heap buffer overflow.

Getting Into the Details

When Bitdefender’s 7z module discovers an EncodedHeader in a 7z archive, it tries to decompress it with the LZMA decoder. Their code seems to be based on 7-Zip, but they made a few changes. Loosely speaking, the extraction of a 7z EncodedHeader is implemented as follows:

Read the unpackSize from the 7z EncodedHeader. Allocate unpackSize bytes. Use the C API of the LZMA decoder that comes with 7-Zip and let it decompress the stream.

The following snippet shows how the allocation function is called:

1DD02A845FA lea rcx, [rdi+128h] //<-------- result 1DD02A84601 mov rbx, [rdi+168h] 1DD02A84608 mov [rsp+128h], rsi 1DD02A84610 mov rsi, [rax+10h] 1DD02A84614 mov [rsp+0E0h], r15 1DD02A8461C mov edx, [rsi] //<-------- size 1DD02A8461E call SZ_AllocBuffer

Recall the x64 calling convention. In particular, the first two integer arguments (from left to right) are passed via rcx and rdx .

SZ_AllocBuffer is a function within the Bitdefender 7z module. It has two arguments:

The first argument result is a pointer to which the result (a pointer to the allocated buffer in case of success or NULL in case of a failure) is written.

is a pointer to which the result (a pointer to the allocated buffer in case of success or in case of a failure) is written. The second argument size is the allocation size.

Let us look at the functions’s implementation.

260ED3025D0 SZ_AllocBuffer proc near 260ED3025D0 260ED3025D0 mov [rsp+8], rbx 260ED3025D5 push rdi 260ED3025D6 sub rsp, 20h 260ED3025DA mov rbx, rcx 260ED3025DD mov edi, edx //<-------- edi holds size 260ED3025DF mov rcx, [rcx] 260ED3025E2 test rcx, rcx 260ED3025E5 jz short loc_260ED3025EC 260ED3025E7 call near ptr irrelevant_function 260ED3025EC 260ED3025EC loc_260ED3025EC: 260ED3025EC cmp edi, 0FFFFFFFFh //<------- {*} 260ED3025EF jbe short loc_260ED302606 260ED3025F1 xor ecx, ecx 260ED3025F3 mov [rbx], rcx 260ED3025F6 mov eax, ecx 260ED3025F8 mov [rbx+8], ecx 260ED3025FB mov rbx, [rsp+30h] 260ED302600 add rsp, 20h 260ED302604 pop rdi 260ED302605 retn 260ED302606 ; ------------------------------------ 260ED302606 260ED302606 loc_260ED302606: 260ED302606 mov rcx, rdi //<------ set size argument for mymalloc 260ED302609 call mymalloc //[rest of the function omitted]

Note that mymalloc is just a wrapper function that eventually calls malloc and returns the result.

Apparently, the programmer expected the size argument of SZ_AllocBuffer to be of a type with size greater than 32 bits. Obviously, it is only a 32-bit value.

It is funny to see that the compiler failed to optimize away the comparison at {*} , given that its result is only used for an unsigned comparison jbe . If you have any hints on why this might happen, I’d be very interested to hear them.

After SZ_AllocBuffer returns, the function LzmaDecode is called:

LzmaDecode(Byte *dest, SizeT *destLen, const Byte *src, SizeT *srcLen, /* further arguments omitted */ )

Note that dest is the buffer allocated with SZ_AllocBuffer and destLen is supposed to be a pointer to the buffer’s size.

In the reference implementation, SizeT is defined as size_t . Interestingly, Bitdefender’s 7z module uses a 64-bit type for SizeT in both the 32-bit and the 64-bit version, making both versions vulnerable to this bug. I suspect that this is the result of an effort to create identical behavior for the 32-bit and 64-bit versions of the engine.

The LZMA decoder extracts the given src stream and writes (up to) *destLen bytes to the dest buffer, where *destLen is the 64-bit unpackSize from the 7z EncodedHeader. This results in a neat heap buffer overflow.

Triggering the Bug

To trigger the bug, we create a 7z LZMA stream containing the data we want to write on the heap. Then, we construct a 7z EncodedHeader with a Folder that has an unpackSize of (1<<32) + 1 . This should make the function SZ_AllocBuffer allocate a buffer of 1 byte.

That sounds nice, but does this actually work?

0:000> g !Heap block at 1F091472D40 modified at 1F091472D51 past requested size of 1 (2f8.14ec): Break instruction exception - code 80000003 (first chance) ntdll!RtlpNtMakeTemporaryKey+0x435e: 00007ff9`d849c4ce cc int 3 0:000> db 1F091472D51 000001f0`91472d51 59 45 53 2c 20 54 48 49-53 20 57 4f 52 4b 53 ab YES, THIS WORKS.

Attacker Control and Exploitation

The attacker can write completely arbitrary data to the heap without any restriction. A file system minifilter is used to scan all files that touch the disk, making this vulnerability easily exploitable remotely, for example by sending an e-mail with a crafted file as attachment to the victim.

Moreover, the engine runs unsandboxed and as NT Authority\SYSTEM . Hence, this bug is highly critical. However, since ASLR and DEP are in place, successful exploitation for remote code execution might require another bug (e.g. an information leak) to bypass ASLR.

Note also that Bitdefender’s engine is licensed to many different anti-virus vendors, all of which could be affected by this bug.

The Fix

The patched version of the function SZ_AllocBuffer looks as follows:

1E0CEA52AE0 SZ_AllocBuffer proc near 1E0CEA52AE0 1E0CEA52AE0 mov [rsp+8], rbx 1E0CEA52AE5 mov [rsp+10h], rsi 1E0CEA52AEA push rdi 1E0CEA52AEB sub rsp, 20h 1E0CEA52AEF mov esi, 0FFFFFFFFh 1E0CEA52AF4 mov rdi, rdx //<-----rdi holds the size 1E0CEA52AF7 mov rbx, rcx 1E0CEA52AFA cmp rdx, rsi //<------------{1} 1E0CEA52AFD jbe short loc_1E0CEA52B11 1E0CEA52AFF xor eax, eax 1E0CEA52B01 mov rbx, [rsp+30h] 1E0CEA52B06 mov rsi, [rsp+38h] 1E0CEA52B0B add rsp, 20h 1E0CEA52B0F pop rdi 1E0CEA52B10 retn 1E0CEA52B11 ; ----------------------------------- 1E0CEA52B11 1E0CEA52B11 loc_1E0CEA52B11: 1E0CEA52B11 mov rcx, [rcx] 1E0CEA52B14 test rcx, rcx 1E0CEA52B17 jz short loc_1E0CEA52B1E 1E0CEA52B19 call near ptr irrelevant_function 1E0CEA52B1E 1E0CEA52B1E loc_1E0CEA52B1E: 1E0CEA52B1E cmp edi, esi //<------------{2} 1E0CEA52B20 jbe short loc_1E0CEA52B29 1E0CEA52B22 xor ecx, ecx 1E0CEA52B24 mov [rbx], rcx 1E0CEA52B27 jmp short loc_1E0CEA52B3B 1E0CEA52B29 ; ----------------------------------- 1E0CEA52B29 1E0CEA52B29 loc_1E0CEA52B29: 1E0CEA52B29 mov ecx, edi 1E0CEA52B2B call near ptr mymalloc //[rest of the function omitted]

Most importantly, we see that the function’s second argument size has been changed to a 64-bit type.

Note that at {1} , a check ensures that the passed size is not greater than 0xFFFFFFFF .

At {2} , the value of rdi is guaranteed to be at most 0xFFFFFFFF , hence it suffices to use the 32-bit register edi . However, just as in the original version (see above), it is useless to compare this 32-bit value once more to 0xFFFFFFFF and it is a mystery to me why the compiler does not optimize this away.

Using a full 64-bit type for the second argument size resolves the described bug.

Conclusion

In a nutshell, the discovered bug is a 64-bit value size being passed to the allocation function SZ_AllocBuffer which looks roughly like this :

void * SZ_AllocBuffer( void *resultptr, uint32_t size);

Assuming that the size is not explicitly casted, the compiler should throw a warning of the following kind:

warning C4244: 'argument': conversion from 'uint64_t' to 'uint32_t', possible loss of data

Note that in Microsoft’s MSVC compiler, this is a Level2 warning (Level1 being the lowest and Level4 being the highest level). Hence, this bug most likely could have been avoided simply by taking compiler warnings seriously.

For a critical codebase such as the engine of an anti-virus product, it would be adequate to treat warnings as errors, at least up to a warning level of 2 or 3.

Nevertheless, the general type of bug shows that even if only few lines of additional code are necessary to incorporate external code (such as the 7-Zip code) into a code base, those very lines can be particularly prone to error.

Do you have any comments, feedback, doubts, or complaints? I would love to hear them. You can find my e-mail address on the about page.

Alternatively, you are invited to join the discussion on HackerNews or on /r/netsec.

Timeline of Disclosure

2017-07-24 - Discovery

2017-07-24 - Report

2017-07-24 - “Thank you for your report, we will investigate and come back with an answer.”

2017-08-22 - “confirm that this vulnerability has been patched” and “will have an internal discussion about the reward”

2017-09-12 - Bug bounty paid

Thanks & Acknowledgements

I want to thank Bitdefender and especially Marius for their response as well as for fixing the bug.