Skip to content

Fix mbregex search state after cache invalidation - #22954

Closed
matthiasgoergens wants to merge 2 commits into
php:PHP-8.4from
matthiasgoergens:codex/gh-21036-php-8.4-20260731
Closed

Fix mbregex search state after cache invalidation#22954
matthiasgoergens wants to merge 2 commits into
php:PHP-8.4from
matthiasgoergens:codex/gh-21036-php-8.4-20260731

Conversation

@matthiasgoergens

@matthiasgoergens matthiasgoergens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Fixes GH-21036.

php_mbregex_compile_pattern() may replace the cache entry currently referenced by search_re. The replacement destroys that regex and clears search_re, but leaves search_regs holding match offsets associated with the destroyed regex. mb_ereg_search_getregs() then treats those registers as valid and dereferences the null regex when processing named captures.

Free and clear search_regs whenever cache replacement clears search_re. This preserves the existing invariant that match registers do not outlive their regex and makes mb_ereg_search_getregs() return false instead of returning stale numeric captures while losing named captures.

PR #21038 identified the crash and proposed guarding the named-capture lookup. This alternative also discards the invalidated match state, following the approach used for the analogous invalid-pattern path in commit 392ad20.

@matthiasgoergens
matthiasgoergens marked this pull request as ready for review July 30, 2026 17:32
@alexdowad

Copy link
Copy Markdown
Contributor

I have very limited time right now. Is @youkidearitai in a position to review this one?

@youkidearitai youkidearitai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your pull request.

I confirmed failed test when before patch this pull request. And I confirmed delete segfault.

=====================================================================
Running selected tests.
TEST 1/1 [ext/mbstring/tests/gh21036.phpt]
========DIFF========
001- bool(false)
001+ Segmentation fault (core dumped)
002+
003+ Termsig=11
========DONE========
FAIL GH-21036 (mb_ereg_search_getregs() after regex cache invalidation) [ext/mbstring/tests/gh21036.phpt]
=====================================================================
Number of tests :     1                 1
Tests skipped   :     0 (  0.0%) --------
Tests warned    :     0 (  0.0%) (  0.0%)
Tests failed    :     1 (100.0%) (100.0%)
Tests passed    :     0 (  0.0%) (  0.0%)
---------------------------------------------------------------------
Time taken      : 0.072 seconds
=====================================================================

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
GH-21036 (mb_ereg_search_getregs() after regex cache invalidation) [ext/mbstring/tests/gh21036.phpt]
=====================================================================

Please add a NEWS that this fixes.

@matthiasgoergens
matthiasgoergens force-pushed the codex/gh-21036-php-8.4-20260731 branch from 6e99a28 to 7d9f78a Compare July 31, 2026 05:16
@matthiasgoergens

Copy link
Copy Markdown
Contributor Author

Thanks @alexdowad and @youkidearitai for the quick turnaround, and thanks for independently verifying the reproducer and fix. I’ve added the requested NEWS entry and folded it into the fix commit.

Comment thread ext/mbstring/tests/gh21036.phpt
@youkidearitai

Copy link
Copy Markdown
Contributor

@matthiasgoergens Thank you very much for your pull request. I merged and closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants