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 decompileIF #76

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

Comments

Projects
None yet
3 participants
@bestshow
Copy link

commented Jun 8, 2017

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

#swftocxx $FILE out
=================================================================
heap-buffer-overflow on address 0x6020000000a0 at pc 0x000000570171 bp 0x7fffe4c1cbe0 sp 0x7fffe4c1cbd8
READ of size 1 at 0x6020000000a0 thread T0
    #0 0x570170 in decompileIF /home/haojun/Downloads/libming-master/util/decompile.c:2369:79
    #1 0x5875eb in decompileActions /home/haojun/Downloads/libming-master/util/decompile.c:3401:6
    #2 0x5875eb in decompile5Action /home/haojun/Downloads/libming-master/util/decompile.c:3423
    #3 0x52a0c5 in outputSWF_DOACTION /home/haojun/Downloads/libming-master/util/outputscript.c:1548:29
    #4 0x531311 in readMovie /home/haojun/Downloads/libming-master/util/main.c:277:4
    #5 0x531311 in main /home/haojun/Downloads/libming-master/util/main.c:350
    #6 0x7f7029da9b34 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/../csu/libc-start.c:274
    #7 0x41ae7b in _start (/home/haojun/Downloads/libming-afl-build/bin/swftocxx+0x41ae7b)

0x6020000000a0 is located 12 bytes to the right of 4-byte region [0x602000000090,0x602000000094)
allocated by thread T0 here:
    #0 0x4dfd76 in malloc /home/haojun/Downloads/llvm-clang/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:66
    #1 0x5d42e0 in readBytes /home/haojun/Downloads/libming-master/util/read.c:228:17
    #2 0x7f7029da9b34 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:2369:79 in decompileIF
Shadow bytes around the buggy address:
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff8000: fa fa 00 fa fa fa 02 fa fa fa 00 04 fa fa 01 fa
=>0x0c047fff8010: fa fa 04 fa[fa]fa 00 03 fa fa 07 fa fa fa 01 fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8060: 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
==15153==ABORTING

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

@hlef

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2017

Hello,

sact->numActions can be 0.

This is problematic because sact->numActions-1 may be used as index when accessing sact->Actions array, for example util/decompile.c:2369.

(Also, it even looks like sact->numActions-2 may be used as index, e.g. util/decompile.c:2246)

I don't know the codebase very well, but is there any reason why ming would process a block with numActions < 1 ?

If not, adding a check to avoid processing these blocks would probably be a conceivable solution.

By the way, this issue was assigned ID CVE-2017-11704.

@strk

This comment has been minimized.

Copy link
Member

commented Sep 30, 2017

@hlef

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2017

I'm working on it, but the following patch is not sufficient.

diff --git a/util/decompile.c b/util/decompile.c
index c844fa49..60785e29 100644
--- a/util/decompile.c
+++ b/util/decompile.c
@@ -2197,6 +2197,11 @@ decompileIF(int n, SWF_ACTION *actions, int maxn)
        int j,i=0;
        struct strbufinfo origbuf;
        OUT_BEGIN2(SWF_ACTIONIF);
+
+        if (sact->numActions < 1) {
+            return 0;
+        }
+
        /*
        * IF is used in various way to implement different types
        * of loops. We try to detect these different types of loops

Another issue follows

==2756==WARNING: AddressSanitizer failed to allocate 0xffffffffa29bda16 bytes
==2756==AddressSanitizer's allocator is terminating the process instead of returning 0
==2756==If you don't like this behavior set allocator_may_return_null=1
==2756==AddressSanitizer CHECK failed: ../../../../src/libsanitizer/sanitizer_common/sanitizer_allocator.cc:147 "((0)) != (0)" (0x0, 0x0)
    #0 0x7ffff6f5eba3 (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x59ba3)
    #1 0x7ffff6f62ae3 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5dae3)
    #2 0x7ffff6f1e7c3 (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x197c3)
    #3 0x7ffff6f61341 (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5c341)
    #4 0x7ffff6f596f0 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x546f0)
    #5 0x435217 in readBytes /home/user/Dev/ming/ming-0.4.8/libming-ming-0_4_8/util/read.c:227
    #6 0x434c09 in parseSWF_UNKNOWNBLOCK /home/user/Dev/ming/ming-0.4.8/libming-ming-0_4_8/util/parser.c:3551
    #7 0x40203e in readMovie /home/user/Dev/ming/ming-0.4.8/libming-ming-0_4_8/util/main.c:266
    #8 0x40203e in main /home/user/Dev/ming/ming-0.4.8/libming-ming-0_4_8/util/main.c:351
    #9 0x7ffff63f3b44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
    #10 0x402846 (/usr/local/bin/swftocxx+0x402846)

A long block is detected, so length is updated at util/main:256 but for some reasons it takes value -1566844394. There's probably an overflow somewhere, or readUInt32 is over-reading, or it could maybe be a cast issue.

Also, length (util/main.c:237) has int type, even if it should be able to store 32 bit values in case of long blocks. Is there a reason for that ? Otherwise, this is probably a bug and be an issue if sizeof(int)=2.

@hlef

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2017

Well, here is the problem:

length (util/main.c:237) has int type, but it should be able to store unsigned 32 bit values. Let's assume readUInt32 returns a correct value (which is not the case). Then the unsigned long result is cast to a signed int, so length might not be valid anymore.

However, changing the type of length would not be enough because the are other issues in readUInt32.

  • readUInt32 is doing (readUInt8(f)<<24), which is undefined behavior for 2 bytes integers. It is also prone to overflows for 4 bytes integers if we don't cast the result of readUInt8 to unsigned long.
  • readUInt32 is calling all readUInt8(f) in one line, but afaik order of evaluation is not guaranteed in the C standard.

For example in our case the fourth call of readUInt8(f) returns 162. Since 162 << 24 = 0xa2000000 = 2717908992 > LONG_MAX, there's an overflow and the result is wrong.

readSInt32 is probably affected by the same weaknesses.

@hlef

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2017

I think this issue can be closed now

@strk

This comment has been minimized.

Copy link
Member

commented Oct 6, 2017

Closed via #88

@strk strk closed this Oct 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.