Repository navigation
Add stack overflow and nesting depth protection to parser - #167
Closed
beltoforion wants to merge 3 commits into
Closed
beltoforion wants to merge 3 commits into
beltoforion wants to merge 3 commits into
Conversation
1. Out-of-bounds read in IsStrVarTok (muParserTokenReader.cpp):
The string-variable buffer index check only tested for a non-empty
buffer, not that item->second was a valid index. Replace with a
proper bounds check: `item->second >= m_vStringVarBuf.size()`.
2. No runtime bounds check on eval-loop stack index `sidx`
(muParserBase.cpp):
- The only guard was a MUP_ASSERT (disabled in release builds).
- Replace the MUP_ASSERT in the cmIF case with a real Error() call.
- Compute iStackMaxSize once and guard every ++sidx push operation
(cmVAR, cmVAL, cmVARPOW*, cmVARMUL, 0-arg cmFUNC/cmFUNC_BULK)
against exceeding the allocated buffer.
3. Unbounded nesting depth (muParserDef.h, muParserTokenReader.cpp,
muParserBase.cpp):
- Add MaxNestingDepth = 1000 constant.
- Enforce it on the bracket stack in IsOprt() so deeply-nested
bracket expressions are rejected instead of consuming unbounded
heap memory.
- Enforce it on the ifElseCounter in CreateRPN() for nested
ternary expressions.
4. Integer overflow in GetMaxStackSize() (muParserBytecode.cpp):
`m_iMaxStackSize + 1` wraps to 0 when m_iMaxStackSize == SIZE_MAX,
causing the stack buffer to be under-allocated. Add an overflow
guard that throws ecINTERNAL_ERROR in that (theoretical) case.
https://claude.ai/code/session_01AdA3xUA3X15KMYv6VC7tXQ
The previous MUP_ASSERT(sidx >= 0) was already a runtime check (MUP_ASSERT always throws ParserError, it is not a debug-only macro). However, it was placed inside the 'if (stack[sidx--] == 0)' body, so a negative sidx was only detected when the condition evaluated to zero. When the condition was non-zero the decrement still happened but the bounds check was skipped. Restructure the check so sidx is validated unconditionally after the post-decrement, regardless of which branch the ternary takes. https://claude.ai/code/session_01AdA3xUA3X15KMYv6VC7tXQ
Instead of restructuring the conditional with an intermediate bool, simply place MUP_ASSERT(sidx >= 0) after the if block so it runs unconditionally on both branches. https://claude.ai/code/session_01AdA3xUA3X15KMYv6VC7tXQ
beltoforion
added a commit
that referenced
this pull request
Apr 10, 2026
Owner
Author
|
closed; ai generated code will not be merged; i added the modifications manually after a review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds runtime safety checks to prevent stack overflow and excessive nesting depth in the muParser expression evaluator. These changes protect against both malicious and accidental inputs that could cause buffer overflows or stack exhaustion.
Key Changes
Stack overflow protection: Added
iStackMaxSizevariable to track the maximum allowed stack size and inserted bounds checks before all stack push operations (cmVAR,cmVAL,cmVARPOW2,cmVARPOW3,cmVARPOW4,cmVARMUL, and function calls with 0 arguments)Nesting depth limits:
MaxNestingDepthconstant (1000) tomuParserDef.hcmIFbytecode generation to prevent excessive if/else nestingError handling improvements:
MUP_ASSERTwith runtimeError()call incmIFcase to handle negative stack indexGetMaxStackSize()to detect invalid stack size valuesCode quality:
#include <limits>to supportstd::numeric_limitsImplementation Details
The stack protection works by calculating the maximum stack size per thread at the beginning of expression evaluation and checking before each push operation. If a push would exceed the limit, an
ecINTERNAL_ERRORis raised.The nesting depth protection prevents deeply nested expressions from consuming excessive memory or causing stack exhaustion during parsing and bytecode generation.
https://claude.ai/code/session_01AdA3xUA3X15KMYv6VC7tXQ