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

libFuzzer integration + bug report #1035

Closed
wants to merge 1 commit into from

Conversation

@yevgenypats
Copy link

commented Jul 7, 2019

This commit places the basics for libFuzzer integration with one
fuzzer which fuzzes the mg_parse_http function. The fuzzer is
located at fuzz/mg_parse_http.

To add more fuzzers please add them to ./fuzz directory.

Also a memory corruption bug is found using this fuzzer which
might lead to additional bugs after fix is pushed.

@yevgenypats

This comment has been minimized.

Copy link
Author

commented Jul 7, 2019

Attached is the crash for the mg_parse_http fuzzer + screenshot of the backtrace.

I think this should also be given a CVE for a memory corruption vulnerability.

I'll be happy to contribute more fuzzers after this bug is fixed so it won't fail the other fuzzers.

Also, I'll be happy to contribute an integration to Fuzzit (I'm the CEO of the company) which will enable continuous fuzzing for the project for free as it's an open-source project. you can read also about systemd case study here.

To reproduce run

cd fuzz
make
./fuzz/mg_parse_http/mg_parse_http

mongoose
crash-c6a21364a4b293c2074130802bb38bd91b054a7c.txt

Cheers,
Yevgeny

@yevgenypats

This comment has been minimized.

Copy link
Author

commented Jul 7, 2019

Not sure who is the right person to review the PR. but CCing @cpq as you merged a CVE bugfix lately.

@yevgenypats

This comment has been minimized.

Copy link
Author

commented Jul 8, 2019

@alashkin maybe you can look at this? not sure who is the right person.

@bobsayshilol

This comment has been minimized.

Copy link

commented Jul 11, 2019

Hi. I saw this posted on reddit and am in no way affiliated with the project, but it looks like you've accidentally committed the 2.7MB fuzzing binary which likely isn't meant to be in the repo.

I had a brief go at running the fuzzer myself and it looks like the crash is the result of an incorrect assumption on the behaviour of atoi(). From the documentation:

The string may begin with an arbitrary amount of white space (as determined by isspace(3)) followed by a single optional '+' or '-' sign.

This patch fixes the issue by checking that the first character of the status code isn't a space, and hence fixes the crash as it can no longer be given a string of 4 spaces which it would read off of:

diff --git a/src/mg_http.c b/src/mg_http.c
index ef546c3..972cbec 100644
--- a/src/mg_http.c
+++ b/src/mg_http.c
@@ -455,7 +455,7 @@ int mg_parse_http(const char *s, int n, struct http_message *hm, int is_req) {
     }
   } else {
     s = mg_skip(s, end, " ", &hm->proto);
-    if (end - s < 4 || s[3] != ' ') return -1;
+    if (end - s < 4 || isspace(s[0]) || s[3] != ' ') return -1;
     hm->resp_code = atoi(s);
     if (hm->resp_code < 100 || hm->resp_code >= 600) return -1;
     s += 4;

I also beefed up the fuzzer locally to test the other paths and nothing else was flagged up. Feel free to use it if you'd like:

int LLVMFuzzerTestOneInput(const uint8_t * data, size_t size) {
    struct http_message req;
    int is_req;

    /* The first byte determines whether or not this is a request */
    if (size < 1) return 0;
    is_req = *data++;
    size--;

    if (is_req != 0 && is_req != 1) return 0;

    mg_parse_http((const char *)data, size, &req, is_req);
    return 0;
}

Also I notice the undefined behaviour sanitizer isn't enabled in the root Makefile? Nothing was flagged up when it was enabled, but integer overflow is a very common issue and that sanitizer is ideal for detecting it.

@cpq

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Thank you gents.
Before it gets integrated, please sign a CLA , linked at https://github.com/cesanta/mongoose#contributions

libFuzzer integration + bug report
This commit places the basics for libFuzzer integration with one
fuzzer which fuzzes the mg_parse_http function. The fuzzer is
located at fuzz/mg_parse_http.

To add more fuzzers please add them to ./fuzz directory.

Also a memory corruption bug is found using this fuzzer which
might lead to additional bugs after fix is pushed.

@yevgenypats yevgenypats force-pushed the fuzzitdev:libfuzzer_integration branch from 7b06ccb to 82beafb Jul 13, 2019

@yevgenypats

This comment has been minimized.

Copy link
Author

commented Jul 13, 2019

@cpq Signed the CLA, Also would you be interested if we also contributed integration to Fuzzit so the fuzzers will run continuously? (like in systemd or apache arrow)

@bobsayshilol Thanks! is there a way to reach out to you privately? wanted to discuss an idea with you? you ping me at yp@fuzzit.dev and I'll reply there.

@bobsayshilol

This comment has been minimized.

Copy link

commented Jul 16, 2019

@cpq note that there aren't any commits attributed to me in this PR so I don't need to sign the CLA, so this should be ready to be looked at/merged.

@yevgenypats I'll drop you an email.

@cpq

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

@yevgenypats @bobsayshilol thank you so much, gents.
The PR went in to the dev branch.

1a38e91

Closing this.

@cpq cpq closed this Jul 19, 2019

@yevgenypats

This comment has been minimized.

Copy link
Author

commented Jul 19, 2019

@cpq You welcome. I wanted to follow-up and see if you guys are interested in integrating the fuzzers with our (Fuzzit) Continuous Fuzzing as a Service platform. My biased opinion:) is that it is crucial for the stability and security of native code applications. Feel free to ping my at yp@fuzzit.dev.

@nluedtke

This comment has been minimized.

Copy link

commented Aug 19, 2019

This was assigned CVE-2019-13503.

@cpq

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

@nluedtke shall the CVE mention Mongoose 6.14 and prior?
6.15 has that bug fixed.

@nluedtke

This comment has been minimized.

Copy link

commented Aug 19, 2019

It does not and nor do I know who reported it. Just referencing it for tracking purposes. Ill mention it on the Red Hat bug.

@cpq

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

Ah, thanks for the clarification @nluedtke , appreciated.

@attritionorg

This comment has been minimized.

Copy link

commented Aug 19, 2019

@cpq can you clarify? the fixing commits don't show they are part of a 6.15 release. at the time this was reported, 6.15 was the current version I think?

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