orangeduck/mpc

Stack overflow with infinite nesting

nmeum opened this issue · 2 comments

nmeum commented

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.

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

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.