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

There is crash in listswf tool of libming . A crafted input can lead to a DoS damage. #85

Closed
owl337 opened this issue Jun 23, 2017 · 5 comments

Comments

@owl337
Copy link

owl337 commented Jun 23, 2017

POC is got from https://github.com/owl337/pocs/blob/master/libswf_POC1.rar

Description:

The debugging information is as follows:

$ ./listswf POC1

there is overflow in function readEncUInt30 that lead to malloc a large size of memory failure. It makes the program crash in pareser.c:3083, the details is below.

parser.c:3080 nsset->Count = readEncUInt30(f);//return 15582466837
parser.c:3081 nsset->NS = malloc(sizeof(U30) * nsset->Count);// malloc failure, return NULL
parser.c:3083 nsset->NS[i] = readEncUInt30(f); //cause NULL pointer DEF

$gdb ./listswf
(gdb) set args POC1
(gdb) r
...
(gdb) bt
#0 parseABC_NS_SET_INFO (f=, nsset=,
f=) at parser.c:3083
#1 parseABC_CONSTANT_POOL (cpool=0x676258, f=0x676010) at parser.c:3191
#2 0x0000000000454d14 in parseABC_FILE (abcFile=0x676250, f=0x676010)
at parser.c:3426
#3 0x00000000004558b4 in parseSWF_DOABC (f=0x676010, length=0)
at parser.c:3481
#4 0x0000000000416672 in blockParse (f=0x676010, length=0,
header=) at blocktypes.c:145
#5 0x0000000000411f79 in readMovie (f=0x676010) at main.c:265
#6 main (argc=, argv=) at main.c:350

Credits:

This vulnerability is detected by team OWL337, with our custom fuzzer collAFL. Please contact ganshuitao@gmail.com and chaoz@tsinghua.edu.cn if you need more info about the team, the tool or the vulnerability.

@epozuelo
Copy link

This has been assigned CVE-2017-9988

@hlef
Copy link
Contributor

hlef commented Oct 10, 2017

AFAIK, both parseABC_NS_SET_INFO and parseABC_CONSTANT_POOL are correctly implementing the specification: Since the number of namespaces contained in an ns_set structure has type UInt30, with each ns being represented by an integer, it might very well be that malloc won't be able to allocate enough memory to store the whole ns_set.

Thus, adding a simple check like the one below might be the only solution not involving a major code refactoring.

index 6a70bb0c..7b64efd7 100644
--- a/util/parser.c
+++ b/util/parser.c
@@ -3079,6 +3079,8 @@ void parseABC_NS_SET_INFO(struct ABC_NS_SET_INFO *nsset, FILE *f)
   int i;
   nsset->Count = readEncUInt30(f);
   nsset->NS = malloc(sizeof(U30) * nsset->Count);
+  if (nsset->NS == NULL)
+    SWF_error("parseABC_NS_SET_INFO: Failed to allocate %lu bytes", sizeof(U30) * nsset->Count);
   for(i = 0; i < nsset->Count; i++)
     nsset->NS[i] = readEncUInt30(f);
 }

What do you think of it ?

I can submit a PR after merge of #90.

@strk
Copy link
Member

strk commented Oct 10, 2017 via email

@hlef
Copy link
Contributor

hlef commented Oct 11, 2017

Agree. Should I wait for you to merge #90 or is it fine to push this patch in the same PR ?

@strk
Copy link
Member

strk commented Oct 11, 2017

#90 merged

hlef added a commit to hlef/libming that referenced this issue Oct 11, 2017
Make sure that nsset->NS isn't dereferenced if malloc failed. In this
case, report error and abort.

This commit fixes CVE-2017-9988 (fixes libming#85).
@strk strk closed this as completed in 447821c Oct 12, 2017
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.

4 participants