mparlak / Flee

Fast Lightweight Expression Evaluator
607 stars 119 forks source link

IIS Process Crash on Equation Evaluate #80

Open mohuk opened 3 years ago

mohuk commented 3 years ago

Thanks for the amazing work.

We are facing an issue in which some expressions are causing the entire IIS process to crash when evaluated. We also noticed that enabling 32 bit compatibility on the AppPool resolves the issue and instead of the crash, we have a proper exception bubbled up. We don't want to run our code in 32 bit, can you think of why such a thing can happen ?

wewerak47 commented 3 years ago

Hi, we have the same problem. If we build and run our application in 64bit mode, the apppool crashes. In 32bit mode we get an exception. We also do not want to run our application in 32bit mode. We can provide our code for further invetigation if needed.

hunkydoryrepair commented 3 years ago

This sounds like a showstopping issue. We are considering using Flee in our Web app hosted under IIS. Running in 32bit isn't an option. What exception occurs under 32bit and is there a call stack? That would help identify where the problem is. I need to know if our app would be vulnerable.

hunkydoryrepair commented 3 years ago

I think I encountered this. There is possible stack overflow in the parsing, which will kill the current process. Possible work around is to do the Compile in a thread with a large stack size. Otherwise, the recursive parser needs to be rewritten to not use recursion.

hunkydoryrepair commented 3 years ago

I've rewritten RecursiveDescentParser as StackParser to do the same thing non-recursively. Running under IIS the stack size is much smaller. I couldn't find an expression that would break in a regular app, but break under IIS at just 200 depth. I don't know what the recursion is so deep, it seems to go about 16 levels deep before reading the first token, so it can reach 200 with a complicated expression, with sub-expression inside sub expression, about 7 levels deep. If letting end user enter their own formulas, there isn't really a way to detect that so I feel the recursive solution is unfit for production code.

If anyone is interested, comment here