WIP: Hardening: guest accesses masked into the guest window; file system paths kept inside their mounts #880

Draft
hckrman101 wants to merge 3 commits from hckrman101/harden-mask-paths into main
hckrman101 commented 2026-11-09 00:04:01 +00:00 (Migrated from github.com)

A save file from someone else is data the game's own code parses. Should that code ever be led astray by a crafted file, the worst it should be able to do is crash the game. Two places let it do more:

1. Guest memory: every access masked into the 512 GB window

Lifted code and the host's GP() turned a 64-bit guest address into a host pointer unchecked (A64_MEM + address), so an address the game's code computed from bad data could reach any host memory.

  • A64_PTR (nx2/rtgen.py) masks the address to the window's 39 bits: an address past the window wraps into guest memory, never outside it. GP() is A64_PTR, so the host's own guest-pointer accesses are masked too.
  • nx2/addr_split.py: an access at a base register plus a constant offset (most of them) masks only the base and keeps the offset as the access's displacement (recomp_<op>_k helpers, generated by rtgen.py). Masking base + offset as a whole cost an add and a mask per access; the split form costs one mask per base.
  • host/hos_main.c reserves 4 GB of inaccessible address space after the window and 1 MB before it, for what an access (or a host copy) starting near an end runs over: a crash, not a write into the host's next mapping.

The game's own addresses (image at 0x80000000, heap from 64 GB, regions down from 512 GB) all lie inside the window and are unchanged. Indirect branches were already limited to lifted function entries.

Cost, Linux, RTX 3090, Kokiri Forest (ft-fast, 45 s frame log, three runs each under the timing lock):

build text size fps (3 runs) CPU ms per frame (3 runs)
main 465 MB 52.2 / 36.3 / 45.4 96 / 113 / 105
whole-address mask 529 MB (+13.8%) 41.1 / 47.0 / 46.7 113 / 106 / 104
this PR (split form) 488 MB (+5.0%) 43.2 / 39.4 / 48.8 97 / 106 / 102

The run-to-run spread at this spot (36-52 fps for the same build) is larger than any difference between the builds; no slowdown shows. A64_NO_WINDOW_MASK (measurement) and A64_NO_ADDR_SPLIT=1 (lift time) turn the two parts off.

Paths reached the host as <mount root>/<rest> unchecked, and the game builds some of them from what it reads. save:/../../x left the save folder, and a save folder unpacked from an archive could hold symbolic links that the game would read and write through (DeleteDirectoryRecursively followed them too).

  • host_path refuses a part inside the mount that is not plain names: .., a name of only dots and spaces (Windows trims ... to ..), a backslash, a colon (drive or stream).
  • It refuses a path through a symbolic link below the mount's folder (the folder itself may be one). On Windows a reparse point counts when its tag is a name surrogate (symbolic links, junctions), so OneDrive placeholders keep working.
  • OpenFile / CreateFile open with O_NOFOLLOW on POSIX (FILE_FLAG_OPEN_REPARSE_POINT in the POSIX CreateFileA).
  • DeleteDirectoryRecursively removes a link it meets, not what it names.

Normal paths behave as before. tests/test_fs_paths.py builds the real nn_fs.c with the POSIX layer and checks normal saves, traversal, absolute parts, separators, colons and links (it fails on the old nn_fs.c).

Tested

  • Linux: the game to play and back out, saves written, caches written; tests/run_all.py --strict (the three failures are the same on main: zstandard not installed, test_cpp_preflight).
  • Windows: built and run by nobody yet. The Windows parts are the guard reservation (VirtualAlloc MEM_RESERVE before and after the window) and the reparse-point check (FindFirstFileA's dwReserved0).
A save file from someone else is data the game's own code parses. Should that code ever be led astray by a crafted file, the worst it should be able to do is crash the game. Two places let it do more: ## 1. Guest memory: every access masked into the 512 GB window Lifted code and the host's `GP()` turned a 64-bit guest address into a host pointer unchecked (`A64_MEM + address`), so an address the game's code computed from bad data could reach any host memory. - `A64_PTR` (nx2/rtgen.py) masks the address to the window's 39 bits: an address past the window wraps into guest memory, never outside it. `GP()` is `A64_PTR`, so the host's own guest-pointer accesses are masked too. - nx2/addr_split.py: an access at a base register plus a constant offset (most of them) masks only the base and keeps the offset as the access's displacement (`recomp_<op>_k` helpers, generated by rtgen.py). Masking `base + offset` as a whole cost an add and a mask per access; the split form costs one mask per base. - host/hos_main.c reserves 4 GB of inaccessible address space after the window and 1 MB before it, for what an access (or a host copy) starting near an end runs over: a crash, not a write into the host's next mapping. The game's own addresses (image at 0x80000000, heap from 64 GB, regions down from 512 GB) all lie inside the window and are unchanged. Indirect branches were already limited to lifted function entries. Cost, Linux, RTX 3090, Kokiri Forest (ft-fast, 45 s frame log, three runs each under the timing lock): | build | text size | fps (3 runs) | CPU ms per frame (3 runs) | |---|---|---|---| | main | 465 MB | 52.2 / 36.3 / 45.4 | 96 / 113 / 105 | | whole-address mask | 529 MB (+13.8%) | 41.1 / 47.0 / 46.7 | 113 / 106 / 104 | | this PR (split form) | 488 MB (+5.0%) | 43.2 / 39.4 / 48.8 | 97 / 106 / 102 | The run-to-run spread at this spot (36-52 fps for the same build) is larger than any difference between the builds; no slowdown shows. `A64_NO_WINDOW_MASK` (measurement) and `A64_NO_ADDR_SPLIT=1` (lift time) turn the two parts off. ## 2. File system: guest paths stay inside their mounts, no symbolic links Paths reached the host as `<mount root>/<rest>` unchecked, and the game builds some of them from what it reads. `save:/../../x` left the save folder, and a save folder unpacked from an archive could hold symbolic links that the game would read and write through (DeleteDirectoryRecursively followed them too). - `host_path` refuses a part inside the mount that is not plain names: `..`, a name of only dots and spaces (Windows trims `...` to `..`), a backslash, a colon (drive or stream). - It refuses a path through a symbolic link below the mount's folder (the folder itself may be one). On Windows a reparse point counts when its tag is a name surrogate (symbolic links, junctions), so OneDrive placeholders keep working. - OpenFile / CreateFile open with `O_NOFOLLOW` on POSIX (`FILE_FLAG_OPEN_REPARSE_POINT` in the POSIX CreateFileA). - DeleteDirectoryRecursively removes a link it meets, not what it names. Normal paths behave as before. `tests/test_fs_paths.py` builds the real nn_fs.c with the POSIX layer and checks normal saves, traversal, absolute parts, separators, colons and links (it fails on the old nn_fs.c). ## Tested - Linux: the game to play and back out, saves written, caches written; `tests/run_all.py --strict` (the three failures are the same on main: zstandard not installed, test_cpp_preflight). - Windows: built and run by nobody yet. The Windows parts are the guard reservation (`VirtualAlloc` MEM_RESERVE before and after the window) and the reparse-point check (`FindFirstFileA`'s `dwReserved0`).
NotRexed commented 2026-11-09 05:30:46 +00:00 (Migrated from github.com)

Thank you for this! It changes how the game runs at a level that needs a maintainer design decision first, so I'm closing it to keep the queue clear: needs maintainer design decision, reopen after discussion. (Masking every guest access touches all lifted code: performance and correctness need a design discussion first.)

Thank you for this! It changes how the game runs at a level that needs a maintainer design decision first, so I'm closing it to keep the queue clear: needs maintainer design decision, reopen after discussion. (Masking every guest access touches all lifted code: performance and correctness need a design discussion first.)
Joey-Magnus commented 2026-11-10 05:18:48 +00:00 (Migrated from github.com)

Tested on Linux with an RTX 4070 Ti, merged locally onto current main (9a2e80e plus #1422 and the others), using the complete v2.0.0+update dump.

1. The build breaks after rebasing, because of the address split. The lift stops with:

oot_patches: the output size places were found [0, 1, 0, 0, 1, 1, 1, 1] times, not once each (OUT_EDITS [0, 2, 3]): the lifted C no longer has the text they match
oot_patches: WARNING: the text speed block was found 0 times

nx2/oot_patches.py matches literal lifted C, for example { uint64_t _b=c->x[21]; int64_t _o=20; recomp_store64(c,(_b+_o),c->x[8]); }, and addr_split.py rewrites exactly that form. These patches (output size, text speed, dynamic resolution, mouse camera) landed after this branch's base. With A64_NO_ADDR_SPLIT=1 the same merge builds, so the split is the only cause. The patterns need the _k forms, or matching that accepts both.

2. host/nn_fs.c conflicts with main's own hardening. guest_path_ok (./.., \, :, control characters, and on Windows trailing dot/space and device names) plus the single snapshot copy of the guest path already cover rel_ok. What this PR adds on top is link_below / is_link / O_NOFOLLOW and the name-surrogate test in delete_tree. I resolved the merge as main's guest_path_ok and snapshot copy, then this PR's link_below after the lookup, then this PR's is_link in delete_tree, which keeps OneDrive placeholders usable.

3. Two test fixtures fail to build after the merge (fixture builds, not behaviour): test_fs_paths.py has no HOS_SPACE_END stub for main's gstr, and main's test_fs_links.py doesn't extract is_link, which delete_tree now calls.

Runtime (the merge built with A64_NO_ADDR_SPLIT=1, so masking on, same options file as main's build, DLSS Quality at 4K output, Hyrule Castle road, no mods):

build fps replay submit memory sync GPU
main 52.8 17.5 ms 4.8 ms 12.8 ms
#880 merged (masked, no split) 54.7 16.8 ms 4.45 ms 12.3 ms

No slowdown that I can measure. In-game saving through System -> Save writes slot_03/progress.sav, caption.sav, option.sav and account.sav, with no fs: refused lines. My test user folder is itself a symbolic link above the save mount, which link_below correctly allows. I haven't tested the split form at runtime, because it doesn't build on main until point 1 is fixed.

Tested on Linux with an RTX 4070 Ti, merged locally onto current `main` (9a2e80e plus #1422 and the others), using the complete v2.0.0+update dump. **1. The build breaks after rebasing, because of the address split.** The lift stops with: ``` oot_patches: the output size places were found [0, 1, 0, 0, 1, 1, 1, 1] times, not once each (OUT_EDITS [0, 2, 3]): the lifted C no longer has the text they match oot_patches: WARNING: the text speed block was found 0 times ``` `nx2/oot_patches.py` matches literal lifted C, for example `{ uint64_t _b=c->x[21]; int64_t _o=20; recomp_store64(c,(_b+_o),c->x[8]); }`, and `addr_split.py` rewrites exactly that form. These patches (output size, text speed, dynamic resolution, mouse camera) landed after this branch's base. With `A64_NO_ADDR_SPLIT=1` the same merge builds, so the split is the only cause. The patterns need the `_k` forms, or matching that accepts both. **2. `host/nn_fs.c` conflicts with main's own hardening.** `guest_path_ok` (`.`/`..`, `\`, `:`, control characters, and on Windows trailing dot/space and device names) plus the single snapshot copy of the guest path already cover `rel_ok`. What this PR adds on top is `link_below` / `is_link` / `O_NOFOLLOW` and the name-surrogate test in `delete_tree`. I resolved the merge as main's `guest_path_ok` and snapshot copy, then this PR's `link_below` after the lookup, then this PR's `is_link` in `delete_tree`, which keeps OneDrive placeholders usable. **3. Two test fixtures fail to build after the merge** (fixture builds, not behaviour): `test_fs_paths.py` has no `HOS_SPACE_END` stub for main's `gstr`, and main's `test_fs_links.py` doesn't extract `is_link`, which `delete_tree` now calls. **Runtime** (the merge built with `A64_NO_ADDR_SPLIT=1`, so masking on, same options file as main's build, DLSS Quality at 4K output, Hyrule Castle road, no mods): | build | fps | replay submit | memory sync | GPU | |---|---|---|---|---| | main | 52.8 | 17.5 ms | 4.8 ms | 12.8 ms | | #880 merged (masked, no split) | 54.7 | 16.8 ms | 4.45 ms | 12.3 ms | No slowdown that I can measure. In-game saving through System -> Save writes `slot_03/progress.sav`, `caption.sav`, `option.sav` and `account.sav`, with no `fs: refused` lines. My test user folder is itself a symbolic link above the save mount, which `link_below` correctly allows. I haven't tested the split form at runtime, because it doesn't build on main until point 1 is fixed.
hckrman101 commented 2026-11-10 20:05:14 +00:00 (Migrated from github.com)

Updated onto main (merge 6e92a01; no force push).

host/nn_fs.c: main now has its own path hardening, so its checks are kept as they are (guest_path_ok, the single copy of the guest path, links deleted rather than followed). The old rel_ok is gone. On top of main's checks, this PR still adds:

  • link_below on the resolved path, so a symbolic link or junction below a mount is refused (mod files exempt, as before);
  • O_NOFOLLOW for game file opens on POSIX;
  • the refused() log, now also for paths guest_path_ok rejects;
  • delete_tree treats only name-surrogate reparse points (symbolic links, junctions) as links, so OneDrive placeholder folders are emptied, not skipped.

Address split (Joey-Magnus's point 1): rewriting lifted C inside emit_function ran before main's oot_patches.py and page-mark grouping, which match the lift's exact text, so the lift failed. addr_split is now a last pass in chunks.edit_c, after the patches and grouping. In the real lift every base + constant access became a _k form, and the required patch check passed. New tests/test_addr_split.py covers the rewritten forms and the pass order, and compiles and runs every _k helper against the whole-address one, including wrap past the window.

Masking coverage: every lifted access, GP(), the exclusive/CAS helpers (including main's newer cas_n) and A64_MARK still go through A64_PTR. The GPU back end's own guest_base() + addr uses stay out of scope.

Tested:

  • tests/run_all.py shows the same failures as main.
  • A full Linux build, then a headless run into play: 45-50 fps, about 2,200 draws a frame, account.sav written, no fs: refused, no crash.

Not tested yet:

  • Windows: the guard reservation and the reparse-point check.
  • An in-game save through the System menu.
  • An fps comparison against main.
Updated onto `main` (merge 6e92a01; no force push). **`host/nn_fs.c`:** `main` now has its own path hardening, so its checks are kept as they are (`guest_path_ok`, the single copy of the guest path, links deleted rather than followed). The old `rel_ok` is gone. On top of `main`'s checks, this PR still adds: - `link_below` on the resolved path, so a symbolic link or junction below a mount is refused (mod files exempt, as before); - `O_NOFOLLOW` for game file opens on POSIX; - the `refused()` log, now also for paths `guest_path_ok` rejects; - `delete_tree` treats only name-surrogate reparse points (symbolic links, junctions) as links, so OneDrive placeholder folders are emptied, not skipped. **Address split (Joey-Magnus's point 1):** rewriting lifted C inside `emit_function` ran before `main`'s `oot_patches.py` and page-mark grouping, which match the lift's exact text, so the lift failed. `addr_split` is now a last pass in `chunks.edit_c`, after the patches and grouping. In the real lift every `base + constant` access became a `_k` form, and the required patch check passed. New `tests/test_addr_split.py` covers the rewritten forms and the pass order, and compiles and runs every `_k` helper against the whole-address one, including wrap past the window. **Masking coverage:** every lifted access, `GP()`, the exclusive/CAS helpers (including `main`'s newer `cas_n`) and `A64_MARK` still go through `A64_PTR`. The GPU back end's own `guest_base() + addr` uses stay out of scope. **Tested:** - `tests/run_all.py` shows the same failures as `main`. - A full Linux build, then a headless run into play: 45-50 fps, about 2,200 draws a frame, `account.sav` written, no `fs: refused`, no crash. **Not tested yet:** - Windows: the guard reservation and the reparse-point check. - An in-game save through the System menu. - An fps comparison against `main`.
Commenting is not possible because the repository is archived.
No description provided.