Implement RF bug #72777 - ensure stack limits on mbstring functions. #3997
Conversation
The patch creates new config: mbstring.regex_stack_limit, which defaults to 100000.
| return onig_search((php_mb_regex_t *)opaque, (const OnigUChar *)str, | ||
| (const OnigUChar*)str + str_len, (const OnigUChar *)str, | ||
| (const OnigUChar*)str + str_len, NULL, ONIG_OPTION_NONE) >= 0; | ||
| OnigMatchParam *mp = onig_new_match_param(); |
There was a problem hiding this comment.
Can this be shared between multiple matches? E.g. in PCRE we only use a single global match context for everything.
There was a problem hiding this comment.
I thought about it, and keeping a per-thread copy of this may be ok, needs to be checked in oniguruma code what it actually is. I think there could be situations though where more than one regexp is being processed at one time (e.g. regex1 calls callback, which invokes regex2) and I am not sure how sharing MatchParam will behave in this case. I'd start with the safe way and if we can optimize later it's ok.
| ?> | ||
| --EXPECT-- | ||
| bool(false) | ||
| bool(false) |
There was a problem hiding this comment.
It's odd that mb_ereg() doesn't throw a warning here, while some of the other functions do. It's a preexisting problem though, but maybe something we should fix.
|
Does that raise minimal onigurama minimal supported version ? |
|
@remicollet bundled one supports the necessary API. Not sure about the standalone one, need to check. If there's no check probably a good idea to add one. |
The patch creates new config: mbstring.regex_stack_limit, which defaults to 100000.
This is a port of #2109 to 7.3 since it has up-to-date oniguruma lib which allows to use thread-safe functions. Ideally should be also backported down to 7.1 but that requires either code change or oniguruma upgrade, so that will come later.