orangeduck / mpc

A Parser Combinator library for C
Other
2.68k stars 295 forks source link

Stack overflow with infinite nesting #113

Open nmeum opened 5 years ago

nmeum commented 5 years ago

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 commented 5 years ago

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

orangeduck commented 5 years ago

I added a recursion limit in ea778d1b8dabace291453b650808a9773a21d382. 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.