orangeduck/mpc

stack and heap overflow is expected with a crafted input

yuweol opened this issue · 6 comments

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

Thanks I will try to check this out.

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).

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.

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.

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?

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.