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 dcputs #80

Closed
bestshow opened this issue Jun 8, 2017 · 5 comments · Fixed by #96
Closed

heap buffer overflow in dcputs #80

bestshow opened this issue Jun 8, 2017 · 5 comments · Fixed by #96

Comments

@bestshow
Copy link

bestshow commented Jun 8, 2017

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

#swftocxx $FILE out
=================================================================
heap-buffer-overflow on address 0x7fe82faff800 at pc 0x00000047d145 bp 0x7ffe86b6c620 sp 0x7ffe86b6bdd0
WRITE of size 45 at 0x7fe82faff800 thread T0
    #0 0x47d144 in __interceptor_strcat /home/haojun/Downloads/llvm-clang/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:491
    #1 0x5459fa in dcputs /home/haojun/Downloads/libming-master/util/decompile.c:104:2
    #2 0x5459fa in decompileIMPLEMENTS /home/haojun/Downloads/libming-master/util/decompile.c:3094
    #3 0x5459fa in decompileAction /home/haojun/Downloads/libming-master/util/decompile.c:3375
    #4 0x5875eb in decompileActions /home/haojun/Downloads/libming-master/util/decompile.c:3401:6
    #5 0x5875eb in decompile5Action /home/haojun/Downloads/libming-master/util/decompile.c:3423
    #6 0x52a0c5 in outputSWF_DOACTION /home/haojun/Downloads/libming-master/util/outputscript.c:1548:29
    #7 0x531311 in readMovie /home/haojun/Downloads/libming-master/util/main.c:277:4
    #8 0x531311 in main /home/haojun/Downloads/libming-master/util/main.c:350
    #9 0x7fe83201bb34 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/../csu/libc-start.c:274
    #10 0x41ae7b in _start (/home/haojun/Downloads/libming-afl-build/bin/swftocxx+0x41ae7b)

0x7fe82faff800 is located 0 bytes to the right of 327680-byte region [0x7fe82faaf800,0x7fe82faff800)
allocated by thread T0 here:
    #0 0x4e012d in realloc /home/haojun/Downloads/llvm-clang/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:79
    #1 0x545984 in dcchkstr /home/haojun/Downloads/libming-master/util/decompile.c:92:9
    #2 0x545984 in dcputs /home/haojun/Downloads/libming-master/util/decompile.c:103
    #3 0x545984 in decompileIMPLEMENTS /home/haojun/Downloads/libming-master/util/decompile.c:3094
    #4 0x545984 in decompileAction /home/haojun/Downloads/libming-master/util/decompile.c:3375

heap-buffer-overflow /home/haojun/Downloads/llvm-clang/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:491 in __interceptor_strcat
Shadow bytes around the buggy address:
  0x0ffd85f57eb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffd85f57ec0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffd85f57ed0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffd85f57ee0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ffd85f57ef0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0ffd85f57f00:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffd85f57f10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffd85f57f20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffd85f57f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffd85f57f40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffd85f57f50: 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
==16605==ABORTING

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

@hlef
Copy link
Contributor

hlef commented Nov 20, 2017

This issue is not reproducible since 8b29e8e. I'll investigate it to make sure it is really fixed.

For the record, this issue was assigned identifier CVE-2017-11732.

@hlef
Copy link
Contributor

hlef commented Nov 23, 2017

Hum, in fact the problem is still here.

First thing I noticed after some debugging in util/decompile.c:

void
dcputs(const char *s)
{
      int len=strlen(s); // len = size of the string, *without \0 character !*
      dcchkstr(len); // Make sure our string buffer is big enough, but len is missing a character !
      strcat(dcptr,s); // Copy the string to the end of the buffer => overflow
      dcptr+=len;
      strsize+=len;
}

This should be easy to fix, something like int len=strlen(s)+1; should do the trick.

I'll submit a PR once we're done with #94. I'll also have to make sure it's enough to fix the CVE.

@hlef
Copy link
Contributor

hlef commented Nov 23, 2017

Oh, right, here is another issue:

static int
decompileIMPLEMENTS(int n, SWF_ACTION *actions, int maxn)
{
	struct SWF_ACTIONPUSHPARAM *nparam;
	int i;
	INDENT;
	puts(getName(pop()));
	printf(" implements ");
	nparam=pop();
	for(i=0;i<nparam->p.Integer;i++) // Iterating over SI32 (= signed long) with int counter => Integer overflow risk
	{
		puts(getName(pop())); // puts (= dcputs) is using internal counters of type int => Buffer overflow risk
	}
	println(" ;");
	return 0;
}

This issue is much trickier to fix because it may involve some non-trivial code refactoring.

hlef added a commit to hlef/libming that referenced this issue Dec 4, 2017
The dcputs function appends passed string at the end of the global string
buffer (dcstr), adapting the buffer's size if necessary.

In order to determine whether a buffer size update is necessary or not,
the size of passed string is retrieved using "int len=strlen(s)", which is
incorrect since strlen returns passed string's size without null character
(should be int len=strlen(s)).

This means that passed string may be strcat-ed to the buffer even if the
buffer it to small to hold it, leading to a heap buffer overflow.

This commit addresses this issue (CVE-2017-11732, closes libming#80).
hlef added a commit to hlef/libming that referenced this issue Dec 5, 2017
The dcputs function appends passed string at the end of the global
string buffer (dcstr), adapting the buffer's size if necessary.

Unfortunately, the strsize variable which holds the global buffer's
size is initialized to 0 in dcinit(), which means that no place for
the \0 character is reserved. Hence, whenever dcputs tries to strcat
a string to the global buffer, a byte may be missing leading to a
heap buffer overflow.

This commit addresses this issue (CVE-2017-11732, closes libming#80).
@hlef
Copy link
Contributor

hlef commented Dec 5, 2017

I have pull requested a fix for the first issue mentioned here. Even if I wasn't completely wrong, the first explanation/fix I provided here wasn't completely right and I had to investigate this issue further.

You'll find more detailed explanations in #96.

Concerning the second issue: Nothing critical, and IMO it's a separate issue not related to CVE-2017-11732. I'll open a separate bug report and investigate it further.

@strk strk closed this as completed in #96 Dec 5, 2017
@strk
Copy link
Member

strk commented Dec 5, 2017 via email

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

Successfully merging a pull request may close this issue.

3 participants