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

null pointer dereference in stackswap #78

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 null pointer dereference read was found in function stackswap .

#swftocxx $FILE out
=================================================================
SEGV on unknown address 0x000000000000 (pc 0x000000545058 bp 0x603000000160 sp 0x7fffce29b5b0 T0)
==17155==The signal is caused by a READ memory access.
==17155==Hint: address points to the zero page.
    #0 0x545057 in stackswap /home/haojun/Downloads/libming-master/util/decompile.c:629:29
    #1 0x545057 in decompileSTACKSWAP /home/haojun/Downloads/libming-master/util/decompile.c:1344
    #2 0x545057 in decompileAction /home/haojun/Downloads/libming-master/util/decompile.c:3159
    #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 0x7fd51244fb34 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)

SEGV /home/haojun/Downloads/libming-master/util/decompile.c:629:29 in stackswap
==17155==ABORTING

testcase : https://github.com/bestshow/p0cs/blob/master/null-ptr-in_stackswap
Credit : ADLab of Venustech

@hlef

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2017

Well, it looks like several things are going wrong here.

First, we are building libming in DEBUGSTACK mode (see decompile.c:23), so peek() is not exiting even if the stack is NULL. Instead of exiting, peek() replaces the NULL stack by a debug variable and continues like nothing happened.

This debug variable is the only element in the stack so when we do Stack->type = Stack->next->type (decompile.c:629) we perform a null pointer dereference.

Obvious fix: Add checks to avoid dereferencing next element if == NULL.

Less obvious question: Why is the stack NULL ?

I see two possibilities:

  1. either the stack is user input, so it is fully normal
  2. or there's another bug somewhere else that corrupted the stack

In both cases, we should probably reject files leading to NULL stacks. It is dangerous and IMO useless to process further because the result is going to be garbage anyways.

Possible additional fix: Build without DEBUGSTACK ?

(It should work because, well, DEBUGSTACK is a debug feature, not a production one ;))

@hlef

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2017

I can submit a PR after the merge of #92.

@strk

This comment has been minimized.

Copy link
Member

commented Oct 21, 2017

#92 merged, thanks!

hlef added a commit to hlef/libming that referenced this issue Oct 23, 2017

Fix null-pointer dereference issue in stackswap.
Avoid processing stackswap when stack only contains one element. In this
case, print a warning if debug mode is enabled, and return cleanly.

This commit fixes CVE-2017-11733 (fixes libming#78).
@hlef

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2017

I have just written a potential fix. This fix only addresses the null pointer dereference issue.

In fact the reproducer seems to also trigger various memory leaks similar to #72, which I'm currently investigating.

Short: When readUInt8 is encountering truncated files errors, it does exit(-1) without free-ing previously allocated memory. I suspect this type of errors to very common in the codebase.

@hlef

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

Hum, I suspect that the broken readSInt16, readUInt16 and readSInt32 functions are not completely innocent here. I'll also have to fix them.

@hlef

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

I have updated the PR, which seems to completely address this issue now. Can you take a look ?

@strk strk closed this in #93 Oct 28, 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.