-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IndexOutOfRangeException in RegexInterpreter caused by not always resizing its stack appropriately #58786
Comments
Tagging subscribers to this area: @eerhardt, @dotnet/area-system-text-regularexpressions Issue DetailsFrom fuzzing, these are 3 distinct stacks, with relatively-reduced patterns. Note, the inputs are almost anything - not specially crafted. the examples below are in the interpreter, but I've seen similar crashes/stacks when using compiled mode. RegexRunner, the base class, maintains several stacks (as managed arrays) and allows its derived classes (RegexInterpreter and CompiledRegexRunner) to push and pop from them. They are responsible for calling EnsureStorage() on RegexRunner to enlarge them if that may be necessary. Both do this in certain places, but not all the places they push to them, and the paths below cause the end of the It's not clear to me what the right fix is here: it seems the author assumed that only certain paths would need the check. The simplest thing would be to remove responsibility from the derived classes and check on every push, if that isn't measurably slower. EnsureStorage() would have to stay, as it's part of the public contract (for precompiled regex assemblies) These bugs are not regressions - they repro in .NET Framework. So clearly, low priority.
|
Actually, I only debugged the first case. It's possible that one or both of the others are due to attempting to read out of bounds, rather than write. It's also conceivable that there are other variations, but these are the only 3 distinct stacks. |
From fuzzing, these are 3 distinct stacks, with relatively-reduced patterns. Note, the inputs are almost anything - not specially crafted. the examples below are in the interpreter, but I've seen similar crashes/stacks when using compiled mode.
RegexRunner, the base class, maintains several stacks (as managed arrays) and allows its derived classes (RegexInterpreter and CompiledRegexRunner) to push and pop from them. They are responsible for calling EnsureStorage() on RegexRunner to enlarge them if that may be necessary. Both do this in certain places, but not all the places they push to them, and the paths below cause the end of the
runtrack
(backtracking) stack to be hit.It's not clear to me what the right fix is here: it seems the author assumed that only certain paths would need the check. The simplest thing would be to remove responsibility from the derived classes and check on every push, if that isn't measurably slower. EnsureStorage() would have to stay, as it's part of the public contract (for precompiled regex assemblies)
These bugs are not regressions - they repro in .NET Framework. So clearly, low priority.
The text was updated successfully, but these errors were encountered: