Skip to content

fix(relinker): bound the entry point collector's header and symbol reads - #2196

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
reyani-dev:fix/relinker-entry-point-bounds
Oct 10, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
reyani-dev:fix/relinker-entry-point-bounds

Conversation

@reyani-dev

@reyani-dev reyani-dev commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

Refs #1666, which asks to review the adjacent table reads for the same arithmetic pattern; this is one such finding.

The CFG/GOT filter (unused-filter=1, which runs on non-PLT references) collects entry points in UnusedNidFilter/EntryPointCollector.cpp. It checked every position as offset + size > file size or <= file size: the program headers, PT_DYNAMIC, the init/fini arrays, the section headers, the section name table, the sh_link header and the symbol entries. With an offset near 2^64 the sum wraps, the check passes, and the unchecked read* helpers memcpy from outside the input. The relinker validates none of these section header fields before this point.

Measured on main (d70b8998), with the read helpers temporarily instrumented to report out-of-range reads:

  • e_shoff = 2^64 - 32 with two section headers reads 8 bytes at offset -8 and 4 bytes at -28, both before the input buffer, and the relink still exits 0.
  • Wrapping can also land on an in-bounds table. With e_shoff = 2^64 - 64 and e_shentsize = 0x840, section 1 is read from file offset 0x800. A SHT_SYMTAB there adds a function that nothing else reaches to the entry points, so the GLOB_DAT reference it loads is kept (CFG/GOT filtering: 1 -> 1). The same happens with an in-bounds section header whose symbol table offset wraps (sh_offset = 2^64 - 24).

The fix checks each position by subtraction (base <= size && offset <= size - base && length <= size - base - offset), the same form RelinkerPipeline uses for RELA entries since #917. Out-of-range tables keep their existing handling: program headers and PT_DYNAMIC still throw, and the best-effort section and array scans still stop.

RelativeRelocationIndex.cpp has the same pattern (pos + RelaEntSize, phPos + 56, segOff + segSz), but it is not reachable: by the time it runs, the pipeline has already rejected a wrapping e_phoff (FileByteOffset out of bounds), PT_DYNAMIC (Dynamic segment out of bounds) and RELA table. I checked by instrumenting its read helpers the same way. So it is left unchanged.

Tested

Windows 11, WinLibs GCC 15.2.0 r7, relinker-only Release build, -DBUILD_TESTING=ON:

  • New entry_point_bounds test (Python, relinker fixture, --windows unused-filter=1), with a GLOB_DAT reference loaded only by a function that only a symbol makes reachable:

    Case main This PR
    no section headers (control) 1 -> 0 1 -> 0
    symbol table in bounds (control) 1 -> 1 1 -> 1
    section header table offset wraps onto it 1 -> 1 1 -> 0
    symbol table offset wraps onto the symbol 1 -> 1 1 -> 0
  • ctest on the relinker-only build: 41/41 passed.

  • tools/check_conventions.py and tools/check_test_registration.py pass, and tools/pr_overlap.py --branch reports no overlap.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

🤖 Generated with Claude Code

@reyani-dev

Copy link
Copy Markdown
Contributor Author

Full Linux and Windows builds and tests for this head (fcba0b0d), using the project's Build workflow. I can't start the upstream manual run, so I ran the same build.yml in my fork through a mirror PR. Like the upstream run, the jobs check out the PR head merged into current main (d70b8998):

Job Result
linux success; full build, ctest: 517/517 passed
windows success; full build, ctest: 508/508 passed
relinker (Linux, Windows, macOS) success

Run: https://github.com/AchrafReyani/AnyPS5/actions/runs/37923946113

@reyani-dev

Copy link
Copy Markdown
Contributor Author

To clarify the note about RelativeRelocationIndex.cpp in the description, given #2221 and #2222: I only checked the wrapping, out-of-file case (offsets near 2^64), which the pipeline rejects before that code runs. #2221 is a different case: a relocation table that stays inside the file but runs past its PT_LOAD segment, and #2222 fixes it. The two PRs touch different files and merge cleanly (git merge-tree).

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Please rebase: core/relinker/CMakeLists.txt conflicts with main now, so the build can't merge it. #2330 changes the same lambda in EntryPointCollector.cpp; whichever of the two lands second will need another rebase.

The CFG/GOT filter's entry point collector checked header and symbol
positions as `offset + size > file size`, which wraps for offsets
near 2^64. A section header or symbol table offset there passed the
check, read bytes before the input buffer, and could reach an
in-bounds table through the wrapped address, adding entry points that
keep relocations alive. Check every position by subtraction instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@reyani-dev
reyani-dev force-pushed the fix/relinker-entry-point-bounds branch from fcba0b0 to b1599a9 Compare October 10, 2026 05:24
@reyani-dev

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (b1599a9). The only conflict was in core/relinker/CMakeLists.txt, where main added the windows_diagnostic_fixture_isolation test next to this PR's entry_point_bounds; both are kept.

Relinker-only build on Windows: ctest passes 56/56, including entry_point_bounds, and check_conventions.py passes. The full Linux and Windows build for this head is running and will be posted here. Noted on #2330: I'll rebase again if it lands first.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@github-actions

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#2561 core/relinker/relinker/tests/test_entry_point_bounds.py

@oneandonlydean
oneandonlydean merged commit e31154a into boykopovar:main Oct 10, 2026
11 checks passed
@reyani-dev

Copy link
Copy Markdown
Contributor Author

Full Linux and Windows builds and tests for this head (b1599a94), using the project's Build workflow. I can't start the upstream manual run, so I ran the same build.yml in my fork through a mirror PR. Like the upstream run, the jobs check out the PR head merged into current main (bc40a6b5):

Job Result
linux success; full build, ctest: 599/599 passed
windows success; full build, ctest: 607/607 passed
relinker (Linux, Windows, macOS) success

Run: https://github.com/AchrafReyani/AnyPS5/actions/runs/38027423754

SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
…ry-point-bounds

fix(relinker): bound the entry point collector's header and symbol reads
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.

2 participants