opcache: bounds-validate relocation offsets and document the trust model - #270
Merged
Merged
Conversation
6 tasks
…ust model Defense-in-depth for the file-cache reader (issue #123). PayloadRelocator turned every stored offset into base + stored and drove its loops off count fields read straight from the payload, without range checks - so a crafted .bin carrying the current build's system_id (a build fingerprint, not an authenticator; adler32 is forgeable) fed into getReflection() was an FFI arbitrary read/write primitive. Now every stored value is validated before it becomes an address the engine walks, in the relocate() (untrusted-input) path: - requireOffset: interior-pointer offsets against [0, memSize] - requireStringOffset: tagged interned-string offsets against [0, strSize), plain string offsets against [0, memSize] - requireSpan / requireCount: scriptOffset and every count-driven element array - hashtable buckets/packed data, literals, arg_info (incl. the arg_info[-1] return slot), vars, type lists, attribute args, class property tables, properties_info_table, class/trait names, trait alias/precedence NUL-terminated arrays and their exclude lists, property hooks, dynamic_func_defs, ast children/nodes, warnings and early bindings A violation throws OpCacheException::malformedPayload - a loud refusal, never an out-of-bounds walk. The derelocate()/serialize() path and the #117 graph ScriptSerializer operate on an already-relocated in-process image and inherit this validation. The tagged-offset bound tracks the string section serialize() just rebuilt (re-pinned from the header str_size), so the relocate() inside derelocate() validates against the section it produced, not the stale size. docs/opcache-binary.md gains a "Trust model" section: .bin input must come from a trusted source; system_id is a build fingerprint (ABI guard), not authenticity; adler32 catches accidental corruption, not tampering; the bounds checks turn a memory-safety catastrophe into a clean exception but are not a licence to load untrusted code. The optional keyed-MAC for distributing protected binaries is described as a deploy-side responsibility and flagged as a possible follow-up, deliberately not built in (key management belongs to the application). BoundsValidationTest proves the loud refusal on a truncated buffer, an out-of-range scriptOffset, a hostile pointer field, a hostile hash count and an out-of-range interned-string offset - and that a well-formed image still relocates and round-trips (no false positives). Run in the debug84 and debug84-zts containers too, where an unguarded out-of-bounds read segfaults loudest: no crashes, all refusals clean. Acceptance evidence: - full default suite (536 tests, baseline skips), --group opcache --fail-on-skipped OK (64 tests, 472 assertions) on host, debug84 and debug84-zts; phpstan level max clean; php-cs-fixer clean Fixes #123 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BDcCQiYqbMkjRPyhWgLL6M
lisachenko
force-pushed
the
claude/opcache-117-graph-serializer
branch
from
August 20, 2026 00:30
6a4bc8f to
08ed9b7
Compare
lisachenko
force-pushed
the
claude/opcache-123-bounds-validation
branch
from
August 20, 2026 00:30
d49bf0f to
7a1f13f
Compare
6 tasks
lisachenko
marked this pull request as ready for review
August 20, 2026 00:45
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.
What this changes
Fixes #123 — final PR of the relocator/serializer chain (stacked on #269).
Every stored offset/count/span is validated in the
relocate()(untrusted-input) path before dereference:requireOffset([0, memSize]),requireStringOffset(tagged[0, strSize)),requireSpan/requireCountacross every count-driven array (buckets, literals, arg_info including the[-1]return slot, vars, type lists, attribute args, property tables, propInfoTable, class/trait names, NUL-terminated alias/precedence arrays + excludes, hooks, dynamic_func_defs, ast, warnings, early bindings). Violations throwOpCacheException::malformedPayload. The tagged bound tracks the sectionserialize()rebuilds, so the re-relocate insidederelocate()validates correctly;serialize()and the #117 graph serializer inherit validation (they operate on the already-relocated image). Covers the payload shapes added across the whole chain.Trust model documented in
docs/opcache-binary.md: a.binmust come from a trusted source;system_idis a build fingerprint, not authenticity; adler32 is an accidental-corruption check, not tamper protection; the bounds checks are defense-in-depth that convert a memory-safety catastrophe into a clean exception, not a licence to load untrusted code. A keyed MAC is noted as a deploy-side responsibility / possible follow-up, deliberately not built in.BoundsValidationTest: truncated buffer, out-of-rangescriptOffset, hostile pointer field, hostile hash count, out-of-range interned offset — all refused; a well-formed image still relocates + round-trips (no false positives). Run in the debug84 + debug84-zts containers (where a bounds miss segfaults loudest) — no crashes, clean refusals.Environment it was verified on
php -v): PHP 8.4.19 (cli) (built: Mar 30 2026 19:28:35) (NTS) + ZTS debug container--enable-debug)? yes — debug84 NTS + debug84-zts, opcache gate 64 each, malformed-payload refusals crash-freeAlso: host 536 tests, opcache gate 64; PHPStan level max clean; cs-fixer clean.
Checklist
8.4)composer testpasses on the matching PHP minorcomposer phpstan(level max) andcomposer cs:checkare greenPayloadRelocator— no baseline additionstools/generator/symbols.phpunchanged — nothing generated touched🤖 Generated with Claude Code
https://claude.ai/code/session_01BDcCQiYqbMkjRPyhWgLL6M
Generated by Claude Code