fix unbounded recursion in the script expression parser#1174
Open
aysha-afrah26 wants to merge 1 commit into
Open
fix unbounded recursion in the script expression parser#1174aysha-afrah26 wants to merge 1 commit into
aysha-afrah26 wants to merge 1 commit into
Conversation
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.
The scripting engine compiles precondition, postcondition, and
<Script>attributes into an AST with a hand-written recursive-descent Pratt parser, where every open parenthesis, unary prefix (-,!,~), and right-hand operand adds oneparseExprframe. Nothing bounds that recursion, so a script value that is a long run of(or!recurses once per character until the native stack is exhausted and the process dies with a SIGSEGV. Those strings come straight from XML attributes and are compiled at tree build and validate time, before any node ticks, so a single crafted tree crashes the executor while it is still loading (a ~30 KB nested script is enough on a default 8 MB stack, and less on Windows). I noticed it because the XML parser already guards against this exact shape withkMaxNestingDepthinVerifyXMLandrecursivelyCreateSubtree, while the script parser those paths feed had no equivalent limit. The fix caps the nesting depth insideparseExprand throws a regular parse error once the limit is passed, matching the XML cap. Ordinary scripts are unaffected: long flat expressions and chained comparisons are parsed iteratively, so only genuinely deep nesting trips the limit, and the added test covers both the rejection and that normal scripts still parse.