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 overflow with infinite nesting #113

Open
nmeum opened this issue Jul 8, 2019 · 2 comments
Open

Stack overflow with infinite nesting #113

nmeum opened this issue Jul 8, 2019 · 2 comments

Comments

@nmeum
Copy link
Contributor

nmeum commented Jul 8, 2019

Since mpc_parse_run isn't tail recursive it is possible to trigger a stack overflow with input languages allowing infinite nesting. An example of such an input language is the one used by examples/maths.c. Consider the following input generation program:

#!/bin/sh

PARENTHESES="${1:-5000}"

i=0
while [ $i -ne "${PARENTHESES}" ]; do
	printf '('
	i=$((i + 1))
done

printf "40 + 2"

while [ $i -ne 0 ]; do
	printf ')'
	i=$((i - 1))
done

printf '\n'

And run maths as:

$ ./generate.sh 500000 > mymaths
$ ./maths < mymaths
((<S> <product>) (((<S> ('+' whitespace)) | (<S> ('-' whitespace))) (<S> <product>))*)
((<S> <value>) (((<S> ('*' whitespace)) | (<S> ('/' whitespace))) (<S> <value>))*)
((<S> (one of '0123456789'+ whitespace)) | ((<S> ('(' whitespace)) (<S> <expression>) (<S> (')' whitespace))))
((<S> ((start of input <#>) whitespace)) (<S> <expression>) (<S> (((newline end of input) | (end of input <#>)) whitespace)))
Segmentation fault

This is somewhat problematic if mpc is used for parsing a network protocol as it potentially allows a denial-of-service. Not sure if there is any easy way to fix this but including some kind of recursion limit would probably be one way of doing it.

@orangeduck
Copy link
Owner

Recursion limit is a good idea. I will try to see if I can find some time to test that out.

@orangeduck
Copy link
Owner

I added a recursion limit in ea778d1. Right now it is controlled by a define in the source code but we could also make it defined by some user specified parameter. Of course I don't think it makes mpc completely secure in the slightest but should be a good first line of defense and might help debugging too.

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