Set a hard limit on input length - #129
Merged
Merged
Conversation
dag-erling
force-pushed
the
des/limits
branch
from
December 22, 2025 17:00
92ace01 to
5c414d8
Compare
dag-erling
force-pushed
the
des/limits
branch
2 times, most recently
from
December 22, 2025 17:50
53416f5 to
18e55c8
Compare
In theory, there is no limit on the size of the input given to our tnfa. In practice, we need to be able to report the boundaries of matches and capture groups, and store checkpoints for backtracking, so the length of our input (or rather, the position of the last character) needs to fit in the data types we use for these purposes: * Matches are reported using `regoff_t`, which we currently define to `int` (although it is usually defined to `off_t` or equivalent in system `<regex.h>`). * TRE internals consistently use `int` rather than `size_t` for position-related information. The practical consequence of this is that inputs larger than `INT_MAX` result in integer overflow, which cause the matching function to either return an incorrect result or crash. This may have been acceptable when TRE was first written and memory sizes were measured in megabytes rather than gigabytes, but that is no longer the case. In the long term, we should switch to `off_t` for `regoff_t` and `size_t` (or `ssize_t` since TRE internals occasionally use -1 as a sentinel) for everything else. For now, we start by switching from `int` to `ssize_t` at the edges and setting a hard limit on the string length. If the length of the input is known in advance, we clip it to the maximum; otherwise, we treat reaching the maximum the same as we would treat encountering a terminating null character.
This new test program attempts to a) compile regular expressions of increasing lengths and b) match a small regular expression against inputs of increasing lengths and verifies that we get the expected result (`REG_OK`, `REG_ESPACE`, or `REG_NOMATCH`) depending on the exact length being tested. This allows us to drop the `toolong` tests in `retest` and reduce `MAXSTRSIZE` back down to only what `retest` itself needs.
dag-erling
force-pushed
the
des/limits
branch
from
December 22, 2025 18:08
18e55c8 to
70e91f9
Compare
Collaborator
Author
|
@laurikari have you had a chance to take a look at this? |
Owner
|
In |
Collaborator
Author
I'm not sure this is a problem. If we assume that |
Owner
Agreed, I was not looking at the full picture. |
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
2 participants
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.
If given input longer than
INT_MAX, we will happily accept it and then crash when we increment our position beyondINT_MAXand it rolls around toINT_MIN. This pull request adds checks to the matching engines to force them to stop before overflowing, cleans up the outer API layers to prepare for a more thorough overhaul of the internals later, and adds tests of both this input limit and the regular expression length limit we introduced earlier.