Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

heap buffer overflow in OpCode (via decompileSETMEMBER) #82

Closed
bestshow opened this issue Jun 8, 2017 · 6 comments
Closed

heap buffer overflow in OpCode (via decompileSETMEMBER) #82

bestshow opened this issue Jun 8, 2017 · 6 comments

Comments

@bestshow
Copy link

bestshow commented Jun 8, 2017

On libming latest version, a heap buffer overflow was found in function OpCode .

#swftocxx $FILE out
=================================================================
heap-buffer-overflow on address 0x612000000170 at pc 0x000000555cee bp 0x7ffcc35fb940 sp 0x7ffcc35fb938
READ of size 1 at 0x612000000170 thread T0
    #0 0x555ced in OpCode /home/haojun/Downloads/libming-master/util/decompile.c:868:37
    #1 0x555ced in decompileSETMEMBER /home/haojun/Downloads/libming-master/util/decompile.c:1699
    #2 0x555ced in decompileAction /home/haojun/Downloads/libming-master/util/decompile.c:3202
    #3 0x5875eb in decompileActions /home/haojun/Downloads/libming-master/util/decompile.c:3401:6
    #4 0x5875eb in decompile5Action /home/haojun/Downloads/libming-master/util/decompile.c:3423
    #5 0x52a0c5 in outputSWF_DOACTION /home/haojun/Downloads/libming-master/util/outputscript.c:1548:29
    #6 0x531311 in readMovie /home/haojun/Downloads/libming-master/util/main.c:277:4
    #7 0x531311 in main /home/haojun/Downloads/libming-master/util/main.c:350
    #8 0x7fa41f1c5b34 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/../csu/libc-start.c:274
    #9 0x41ae7b in _start (/home/haojun/Downloads/libming-afl-build/bin/swftocxx+0x41ae7b)

0x612000000170 is located 40 bytes to the right of 264-byte region [0x612000000040,0x612000000148)
allocated by thread T0 here:
    #0 0x4dff2d in calloc /home/haojun/Downloads/llvm-clang/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x5beb8c in parseSWF_DOACTION /home/haojun/Downloads/libming-master/util/parser.c:2428:3
    #2 0x7fa41f1c5b34 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/../csu/libc-start.c:274

heap-buffer-overflow /home/haojun/Downloads/libming-master/util/decompile.c:868:37 in OpCode
Shadow bytes around the buggy address:
  0x0c247fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff8000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fff8010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c247fff8020: 00 00 00 00 00 00 00 00 00 fa fa fa fa fa[fa]fa
  0x0c247fff8030: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fff8040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff8050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff8060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==17016==ABORTING

testcase : https://github.com/bestshow/p0cs/blob/master/heap-buffer-overflow-in_OpCode_by_decompileSETMEMBER
Credit : ADLab of Venustech

@hlef
Copy link
Contributor

hlef commented Oct 2, 2017

Calling OpCode(actions, n-1, maxn) is quite common in util/decompile.c to check the opcode of the previous operation, for example:

OpCode(actions, n-1, maxn) == SWFACTION_STOREREGISTER )

However calling Opcode with n < 1 and especially n = 0is not a really good idea at the moment because n - 1 is used as index when accessing actions array, without checking for <1 values.

This issue is not only present in decompileSETMEMBER, but almost everywhere in util/decompile.c.

There are two obvious solutions to me:

  1. patch all decompile methods to avoid calling Opcode with n < 1
  2. patch Opcode to return a dummy Opcode when n < 1. I'd not expect it to break the decompile* functions, and this patch would be super easy.

I'd rather go for 2).

Such a patch would probably fix #81 and #79 (and maybe many, many other issues) at the same time.

By the way, this vulnerability was assigned ID CVE-2017-11728.

@strk
Copy link
Member

strk commented Oct 2, 2017 via email

@hlef
Copy link
Contributor

hlef commented Oct 3, 2017

Hum, yes we could print a warning, but rather for debug purpose ? I'm not sure it would be interesting information for end users.

I'll submit another PR after #88 is merged.

@hlef
Copy link
Contributor

hlef commented Oct 3, 2017

Hum, looks like there's another problem.

In parseSWF_DEFINESOUND, end variable is defined as fileOffset + length (= 46 + 6 = 52 in our case). Then we read some bytes using readUInt16 and readBits, so fileOffset gets incremented (to 53 in our case). At the end, we read the remaining bytes (= -1) and put them in the data container with readBytes(f, end - fileOffset).

This is fine as long as end - fileOffset >= 0. Otherwise readBytes runs malloc(sizeof(char)*size) with negative size, which, once cast to size_t, results in

AddressSanitizer failed to allocate 0xffffffffffffffff bytes

I'd suggest adding a check in readBytes, so that size < 0 produces the same result as size = 0.

@strk
Copy link
Member

strk commented Oct 3, 2017 via email

@hlef
Copy link
Contributor

hlef commented Oct 5, 2017

Fine, I'll submit a PR after #88 is closed.

(looks like this patch also fixes #81 and #79, indeed.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants