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

A buffer overflow error in mg_get_http_header #1135

Closed
BushraAloraini opened this issue Aug 12, 2020 · 4 comments · Fixed by #1138
Closed

A buffer overflow error in mg_get_http_header #1135

BushraAloraini opened this issue Aug 12, 2020 · 4 comments · Fixed by #1138

Comments

@BushraAloraini
Copy link

A buffer overflow error in mg_get_http_header function in mongoose/src/mg_http.c in Mongoose 6.18, where header_names and header_values have a bound of (MG_MAX_HTTP_HEADERS); however, there is no check to ensure that the loop does not exceed the upper bound. A Maliciously crafted http header can trigger this bug. To fix it, modify the loop condition into:

for (i = 0;i < MG_MAX_HTTP_HEADERS && hm->header_names[i].len > 0 ; i++)

@rojer
Copy link
Collaborator

rojer commented Aug 19, 2020

fix sent, thank you!

@cpq cpq closed this as completed in #1138 Aug 19, 2020
@rojer
Copy link
Collaborator

rojer commented Aug 20, 2020

actually, this will not happen in practice because mg_http_parse_headers will not populate the last slot:

while (i < (int) ARRAY_SIZE(req->header_names) - 1) {

but it's good to have a range limit too.

@BushraAloraini
Copy link
Author

Thank you, rojer, for your response. This bug has been detected using a static analysis tool that is part of a research study. Hence, your input is very important. Actually, the static analysis tool was run with an older version of mongoose, which was used with an Android app (AdAway); at that version, mg_get_http_header is defined in mongoose.c. I just realized that mg_get_http_header is still defined in mongoose.c with the newest version (I thought the definition was moved to a new file). I see that mg_get_http_header that is defined in mongoose.c has been fixed, which is great. Also, I see that mg_get_http_header has been called 22 times in mongoose.c file. Hence, there are many execution paths, so some might trigger the error.

@cpq
Copy link
Member

cpq commented Sep 19, 2020

@BushraAloraini so you ran an analysis tool that spotted a false positive, and logged a https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-25756 ?
I guess the assignment was to log a CVE report.
What static analysis tool was that?

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