Skip to content

fix(libSceSystemService): contain converted executable paths - #5158

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
AaravMehta-07:codex/systemservice-contained-exec
Oct 14, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
AaravMehta-07:codex/systemservice-contained-exec

Conversation

@AaravMehta-07

@AaravMehta-07 AaravMehta-07 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

Keep the resolved secondary converted-executable path lexically beside the running converted executable. Guest paths are split on /, but Windows also interprets backslashes as separators; a name containing ..\ can otherwise escape the sibling executable directory after passing the guest-path checks.

Check the normalized parent before probing the target or running shutdown callbacks. Keep Unix backslashes as ordinary filename characters and preserve the existing flattened-name conversion and main-executable handling. This is a lexical path-boundary fix, not a claim of complete symlink or filesystem sandboxing.

The Windows regression copies only its own test executable to a unique parent-directory fixture. The probe registers a shutdown callback that exits with code 86 before StartHostExecutable, so it proves the unsafe path is accepted on baseline without actually launching any outside executable. Child waits are bounded and CTest has a 20-second timeout.

Tested

Windows, MinGW-w64 GCC 15.2.0, default bundled-dependency Release full-mode CMake configuration with testing enabled, main 0f2683cb7.

  • Actual unpatched API probe reaches the shutdown callback and returns 86; the rejecting-path test fails. The callback prevents an outside launch. The benign copied fixture was removed afterward.
  • With the guard, all five focused CTests pass in 0.56 seconds: the new path-boundary case, both SystemExit cases, and both SystemService cases.
  • The complete Windows all-target build and libs target succeeded. Afterward all six focused cases passed in 0.42 seconds, adding patched_libraries to the five cases above.
  • Rebased again onto main d5835a613 after upstream's libc error-category change. The complete Windows all-target build and libs target succeeded again; all eight post-rebase focused CTests passed in 0.76 seconds, including the new generic-category and Ampr C++ error tests.
  • Conventions, diff check and complete live open-PR overlap scan passed.

The full CTest suite, PS5 hardware and commercial titles were not run. No performance claim.

Related Work

#5083 changes the outer LoadExec implementation but leaves PrepareLaunch unchanged. This fix changes the path-preparation helpers, not that export's body. The complete hunk/export/new-file overlap scanner reports no overlap, including with #5083. The initial broad source-file overlap concern was therefore cleared after exact checking. Neither change semantically requires the other; I will rebase if later changes introduce a conflict.

Checklist

  • Based on current main; no other open PR implements this path-boundary guard
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw; no silent stubs added
  • No notes, investigation files or images added to the repository
  • No new third-party code
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • AI-assisted: yes; investigation, implementation, tests and description. Root reviewed the patch and runtime evidence.

@MarcZX10 MarcZX10 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.

I reviewed the path construction and regression. The guard runs after converting the guest path but before checking the target or running shutdown callbacks, and the Windows test exercises the ..\\ separator case while preventing an outside executable from starting. The change is explicitly scoped to lexical containment, and the reported Windows focused tests/full build cover the new path. I did not find a blocking issue in this diff.

@AaravMehta-07
AaravMehta-07 force-pushed the codex/systemservice-contained-exec branch from ad06e95 to 2f45380 Compare October 13, 2026 13:13
@xXPBSXx

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Windows test: head 2f453805a merged onto current main ef42c5f2a, together with 9 other open pull requests in one build (the merges were conflict-free); a failure would have been bisected per PR. Windows 10, MinGW GCC (WinLibs), Ninja Release, -j2: full build 0 errors; ctest for the tests this change adds or touches: guest_system_service_path_boundary passed; python tools/check_conventions.py --base upstream/main passed for the combined tree. I did not run the new tests against main without the change.

AI-assisted (Claude Code).

@oneandonlydean
oneandonlydean merged commit 094a34c into boykopovar:main Oct 14, 2026
8 of 9 checks passed
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.

4 participants