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 vulnerability in newVar3 (util/decompile.c:517) #118

Closed
fantasy7082 opened this issue Mar 7, 2018 · 2 comments
Closed

Comments

@fantasy7082
Copy link

Hi, i found a null pointer dereference bug in the libming 0.4.8. It crashed in function newVar3.the details are below(ASAN):

./swftocxx 010-NULL-ptr-swf /dev/null 
#include <mingpp.h>


main(){
SWFMovie* m = new SWFMovie(48);

Ming_setScale(1.0);
m->setRate(48.187500);
m->setDimension(3992, 3680);

// SWF_SETBACKGROUNDCOLOR 
m->setBackground(0x30, 0x30, 0x30);

// SWF_DEFINESPRITE 

	//  MovieClip 12336 
SWFMovieClip* character12336 = new SWFMovieClip(); // 12336 frames 

// SWF_END 

// SWF_EXPORTASSETS 
m->addExport(character12336,"0000000000000000000");
m->writeExports();

// SWF_INITACTION 
// Might be more appropriate to use addInitAction here
m->add(new SWFInitAction(ASAN:SIGSEGV
=================================================================
==25695==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fe6f0772746 bp 0x7ffc3c2b8f20 sp 0x7ffc3c2b86a8 T0)
    #0 0x7fe6f0772745 in strlen (/lib/x86_64-linux-gnu/libc.so.6+0x8b745)
    #1 0x7fe6f134a1a5 in __interceptor_strlen (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x701a5)
    #2 0x411d16 in newVar3 /root/libming-asan/util/decompile.c:517
    #3 0x41662d in decompileSETMEMBER /root/libming-asan/util/decompile.c:1701
    #4 0x41e5d7 in decompileAction /root/libming-asan/util/decompile.c:3220
    #5 0x41eba0 in decompileActions /root/libming-asan/util/decompile.c:3419
    #6 0x41b07e in decompileIF /root/libming-asan/util/decompile.c:2581
    #7 0x41e715 in decompileAction /root/libming-asan/util/decompile.c:3260
    #8 0x41eba0 in decompileActions /root/libming-asan/util/decompile.c:3419
    #9 0x41eccd in decompile5Action /root/libming-asan/util/decompile.c:3441
    #10 0x40d221 in outputSWF_INITACTION /root/libming-asan/util/outputscript.c:1860
    #11 0x40e331 in outputBlock /root/libming-asan/util/outputscript.c:2083
    #12 0x40f3d9 in readMovie /root/libming-asan/util/main.c:286
    #13 0x40fb0e in main /root/libming-asan/util/main.c:359
    #14 0x7fe6f070782f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #15 0x401b58 in _start (/usr/local/libming-asan/bin/swftocxx+0x401b58)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ??:0 strlen
==25695==ABORTING

POC FILE:https://github.com/fantasy7082/image_test/blob/master/010-NULL-ptr-swf

@hlef
Copy link
Contributor

hlef commented May 15, 2018

Reproducible on latest master, I'll work on a patch.

@hlef
Copy link
Contributor

hlef commented May 25, 2018

Hum, here getString (indirectly called by getName) is passed a variable of type 10 (= "PUSH_VARIABLE") which seems to be a special Ming extension. I couldn't find anything about it in any documentation/specification. What it does it pretty unclear to me.

However the code is developed such that getName should always return a valid string. And, if the specification is implemented correctly there's no reason why getName would return invalid strings. Also, the result of getName should be free-able but with the current code it is pretty unclear to me whether it is the case or not.

I propose the following fix: If the variable contains an invalid string, act like in the PUSH_STRING case. Otherwise return a copy of the string. This doesn't change the behavior of the function and makes sure we avoid returning NULL strings when getting invalid input.

diff --git a/util/decompile.c b/util/decompile.c
index e9341356..5f1e7d6f 100644
--- a/util/decompile.c
+++ b/util/decompile.c
@@ -387,7 +387,14 @@ getString(struct SWF_ACTIONPUSHPARAM *act)
        case 12:
        case 11: /* INCREMENTED or DECREMENTED VARIABLE */
        case PUSH_VARIABLE: /* VARIABLE */
-               return act->p.String;
+               if (!act->p.String)
+               {
+                       SWF_warn("WARNING: Call to getString with PUSH_VARIABLE defining NULL string.\n");
+                       break;
+               }
+               t=malloc(strlen(act->p.String)+1); /* NULL character */
+               strcpy(t,act->p.String);
+               return t;
        default: 
                fprintf (stderr,"  Can't get string for type: %d\n", act->Type);
                break;

I'll submit this patch in the next PR.

hlef added a commit to hlef/libming that referenced this issue May 26, 2018
getString (indirectly called by getName) is passed a variable of non
standard type 10 (= "PUSH_VARIABLE"), which seems to return the
string contained in passed variable, without quotes. If contained
string is NULL, a NULL pointer is returned, which later causes NULL
pointer dereference.

In this patch we address this issue such that if the variable contains
an invalid string, we act just like in the PUSH_STRING case. Otherwise
a copy of the string is returned.

Fixes: libming#118 (CVE-2018-7866).
@strk strk closed this as completed in 0aab70a Jul 12, 2020
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

2 participants