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

stack and heap overflow is expected with a crafted input #117

Open
yuweol opened this issue Aug 26, 2019 · 6 comments
Open

stack and heap overflow is expected with a crafted input #117

yuweol opened this issue Aug 26, 2019 · 6 comments

Comments

@yuweol
Copy link

yuweol commented Aug 26, 2019

Hello, I found two bugs when I called the APIs of mpc library with crafted input value.
I made this codes to replay the bugs by referring test code in mpc repository.
This bugs can be replayed 0.9.0 version.

You can find 4 files in an attached zip file.
bug1.c and bug2.c is the source code to replay these bugs.
bug1.log and bug2.log are runtime error log that address sanitizer informs.

You can regenerate this bug by following commands with bug1.c and bug2.c

[1] clang -m32 -ansi -pedantic -O3 -Wall -Wextra -Wformat=2 -Wshadow -Wno-long-long -Wno-overlength-strings -Wno-format-nonliteral -Wcast-align -Wwrite-strings -Wstrict-prototypes -Wold-style-definition -Wredundant-decls -Wnested-externs -Wmissing-include-dirs -Wswitch-default -fsanitize=address bug1.c mpc.c -o bug1 && ./bug1
[2] clang -m32 -ansi -pedantic -O3 -Wall -Wextra -Wformat=2 -Wshadow -Wno-long-long -Wno-overlength-strings -Wno-format-nonliteral -Wcast-align -Wwrite-strings -Wstrict-prototypes -Wold-style-definition -Wredundant-decls -Wnested-externs -Wmissing-include-dirs -Wswitch-default -fsanitize=address bug2.c mpc.c -o bug2 && ./bug2

I hope this report help mpc to be more secured.

bugs.zip

@orangeduck
Copy link
Owner

Thanks I will try to check this out.

@orangeduck
Copy link
Owner

Hi @yuweol,

For the first bug - the problem is that your input language string talks about a parser called expressionss (encoded as Hex) but none of the parsers you declare have this name. The closest is the parser called expression. If you add NULL to the end of your list of parsers then this error will be correctly checked for and caught (no segfault):

  mpca_lang(MPCA_LANG_DEFAULT,
    input,
    Expr, Prod, Value, Maths, NULL);

Unfortunately this is the only way to do this due to the limitations of using variable arguments.

For the second bug - you have a null terminator in the middle of the string before the "asdmlm dasd" bit appears so all mpc will see is all the newline characters and it will think the string ends there. Once this get trimmed it just becomes the empty string (all newlines are trimmed).

@yuweol
Copy link
Author

yuweol commented Sep 1, 2019

Thanks.

for bug1, I think it would be better to fix at line 74 in tests/grammar.c same as the other usages of mpca_lang function in same code file. I think it will help to understand how to use mpca_lang function correctly.

for bug2, I think even if it is not correct string, it would be better to catch such input in your function before segmentation fault happened.

@orangeduck
Copy link
Owner

Okay we can update the documentation and examples to include the null at the end.

For the second I couldnt reproduce any segmentation fault - I just got the wrong result in the test. For the test result it's impossible to "fix" or even detect - if you put a null in a string this is the end of the string according to C.

@yuweol
Copy link
Author

yuweol commented Sep 1, 2019

It's strange. I have just done it again and segmentation fault is reproduced. Did you try exactly same compiler and compiler option to build the buggy code?

@orangeduck
Copy link
Owner

Okay I managed to reproduce it with your exact command and pushed up a fix in 5120609. That was a particularly nasty edge case since it only happens if you try to trim a string which is completely made of trimmable characters and it seems gdb did not flag it.

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