fix: hardening part 2, guest sizes and indices in the OS layer and renderer #1422

Merged

hckrman101 merged 18 commits from security-hardening-2 into main

2026-11-10 02:31:40 +00:00

hckrman101 commented

2026-11-10 01:40:11 +00:00

(Migrated from github.com)

A second pass after #1398. It checks that pass's fixes for ways around them, and covers the HLE services, the renderer's
copies and draws, and the files players share. Each commit is one topic.

Ways around the first pass

fix(fs): read guest paths once, within bounds; more Windows device names

host_path copies the guest path once, then checks and uses the copy. Another guest thread could change the path
between the check and the use.
gstr gives "" for a path that does not end within 0x302 bytes of guest memory.
guest_path_ok also refuses COM¹-COM³ and LPT¹-LPT³ (UTF-8, the game's code page), CLOCK$, and a device
name followed by spaces (NUL .txt).
DeleteDirectoryRecursively on a folder that is itself a link removes only the link. The posix layer's
GetFileAttributesA marks links as reparse points, as Windows does.
Tested: tests/test_fs_links.py: new cases for the link attribute, and guest_path_ok built as Windows builds it
(device names).

fix(nvn): check the whole of each array the game passes

P(i) checks one byte. The arrays copied with a size now use PN(i, n): border colours, viewports, vertex states,
buffer ranges, render target lists, window textures, push constants, blend colour.

fix(nvn): bound ReadTexels and WriteTexels to the game's buffer

The strided calls take 64-bit row and image strides from the game. Only the buffer's first byte was checked, and the
renderer read or wrote at data + k * img + y * row: anywhere in the process.
The span the strides reach is now computed with overflow checks and must lie in guest memory. The texture, view and
region objects are checked whole.
Tested: tests/test_nvn_host_tables.py: extents, strides that wrap, a stride past the window.

fix(saves): also refuse Windows junctions on Python before 3.12

os.path.isjunction exists from Python 3.12 only, and copytree walks into a junction as into a folder. A link is now
any reparse point or symbolic link, from os.lstat.
Tested: tests/test_oot_saves.py test_junction_refused (a junction simulated through os.lstat). It fails on the
old check.

fix(env): hookshot_env.txt names no libraries, files to write or shares

SDL names that choose a library (SDL_VULKAN_LIBRARY, SDL_VIDEO_EGL_DRIVER, ...) are left out. A bare file name is
looked for beside the game and in the current folder on Windows.
Names ending in _FILE or _DIR, RX_VK_FRAME_LOG and HOS_PRESENT_TRACE take no value from this file. A bare name
resolves in the game folder: HOS_SAMPLE_FILE=hookshot_vk.dll emptied the renderer.
RX_VK_DLSS_DIR may not be a share (\\server\x, //server/x). . and .. count as paths.
HOS_ENV_FILE's file still sets anything. The readme says so.
Tested: tests/test_env_file.py, new lines for each case.

fix(update): no file links in a downloaded Linux list; downloads read at most what they expect

file:// part links are taken only from a list on this PC (the tools' own, HOOKSHOT_LINUX_LIST). In a downloaded
list they could read /dev/zero until memory ran out, or test whether a local file has a given hash.
A download reads at most 4 GB.
Tested: tests/test_linux_update.py: the remote case and the limit.

HLE services

fix(video): check a decoder input's size and data

The submit size is 64-bit and was cut to an int for av_new_packet, then copied in whole: a heap overflow. A size
past INT_MAX is refused, and the data must lie in guest memory.

fix(audio): bound memory pool handles, microphone rings, reverb delays and Opus buffers

Memory pool handles are checked against the 1024-entry table. ReleaseMemoryPool wrote, and the getters read, past it.
An input stream holds at most 10 s. The ring's byte offsets are ints, and a large frame count overflowed them, which
led to writes before the ring.
ReadInputStream writes only into guest memory.
I3DL2 reverb delays are clamped as doubles and from both sides. A negative late delay read before the delay line.
Opus decode: both buffers lie in guest memory, and the frame count is at most 120 ms.
Tested: tests/test_audio_in.py: a stereo stream of 0x20000001 frames. The old code crashes on it.

fix(fs): refuse a mount when the mount table is full

A 17th mount name was written past mounts[16]. It now fails with a mount error.

fix(nvdrv): keep channel sync point ids inside the table

A channel's sync point id grew with each GPFIFO allocation and indexed syncpt[192] unchecked. Ids are used again
after the last one.

fix(mem): check the guest ranges of heap and page calls

SetMemoryHeapSize and svc SetHeapSize check the size without wrap-around. A large size passed the old check, and a
later shrink zeroed memory past the window.
AllocateMemoryPages and FreeMemoryPages refuse ranges outside the guest window. A wrapped range was zeroed whole.
hos_commit and discard_pages act on guest pages only. On Linux they change protection, or discard, whatever range
they get.

fix(os): message queue storage, calendar on bad times, vsync event slots

A message queue whose storage could not be allocated was written through NULL; it now stops with a message.
ToCalendarTime on a time the C library cannot convert returns zeros, not stack bytes.
GetDisplayVsyncEvent takes its slot under a lock. Two threads could write past the 8 slots.

fix(windows): load XInput and IMM32 from System32

LOAD_LIBRARY_SEARCH_SYSTEM32, as for vulkan-1.dll.

Renderer

fix(vk): bound copies, buffer binds and indirect draws by what the game names

The CPU copy from a buffer into a texture (ASTC) checks that its rows fit in what is mapped. It read past the mapping
and past the guest window.
Copy row strides: the buffer size is computed with overflow checks, and a stride Vulkan cannot express is refused.
Before, a wrapped size checked one row while Vulkan copied many.
Copy layer ranges are 64-bit. A range near 2^32 wrapped past the clamp.
gpu_buffer_bind's fast path checks the size without wrap-around, as pool_of does.
Indirect draws bind the records at the stride they are read with. A negative count or stride is no draw. The mesh
fallback draws only the records it converted.
Quad draws over 16M vertices are left out. The host buffers grew with the count, up to 16 GB.
Small: the CPU buffer clear writes at most its 256-byte pattern; packaged block heights and depths are at most 32
GOBs; RX_VK_MESH_ARGS no longer divides by zero.

fix(vk): check the stage and slot of each kept texture answer

hookshot_programs.bin and the pipeline state list name a stage and a descriptor slot for each texture answer. They
were used as indices unchecked, on every draw that found the record.
A cache record whose answers do not fit is left out and translated again. State list answers that do not fit are not
used.
Tested: tests/test_program_cache_cleanup.py: an answer past the stages and an answer past the slots, both
refused, their modules released. tests/test_program_answer_domains.py extracts the new helper.

fix(mods): do not walk a mod's romfs folder that is a link

Links to folders inside a mod were already left out. The romfs folder itself could be one (to /), and the index
walked the whole disk.
Tested: tests/test_fs_links.py.

fix(builder): refuse an update tag that names no folder of its own

The Windows builder cleans the tag into a folder name under builders\ that the update replaces. .. survived the
cleaning. An empty name, a leading dot, or more than 128 characters is now refused, as the setup and the Linux lists do.
Not built here (no .NET on this machine); CI builds it.

ci: keep the release token off redirects

release_parts.get sends GH_TOKEN with add_unredirected_header. urllib copies plain headers to a redirect, which
may go to another host.

How it was tested

python -I -B tests/run_all.py: 203 passed, 5 skipped (no tool for them here).
Linux build of the game and hookshot_vk.so: passes, and the vk ABI check is clean.
Windows: hookshot_vk.dll cross-built with llvm-mingw; a syntax check of every changed host file with llvm-mingw's
clang.
Game run on Linux, in a headless compositor, from a copy of a save:
- the game boots, loads the save and plays (walking, camera, a sword swing);
- the Insert menu opens, moves and closes;
- three runs, two of them first starts that prepared the shaders;
- no error lines in the log.

Not in this PR

scripts/linux/deps.sh downloads dav1d and the Vulkan, SPIR-V and VMA headers as tag archives without a hash (GitLab's
are not byte-stable). Pinning them by commit archive or release tarball needs a decision.
The Linux release job uses ubuntu:22.04 by tag and the toolchain PPA's g++-13 unpinned; its libstdc++ is linked
statically into the shipped renderer.
The program cache still passes its SPIR-V to the driver unvalidated. A cache from someone else is trusted once its
stamp matches. An install key in the stamp, or a MAC, would close that.
fonts/regular.ttf and bold.ttf beside the game go to stb_truetype on Linux, which is not safe for untrusted
fonts (a fuzzer finds out-of-bounds reads within a minute). The ImGui menu work replaces that path.
The Windows game itself does not call SetDefaultDllDirectories. Every load found here is now by full path or from
System32, but SDL's own loads still use the default order.
nx2/nup.py extraction has no output cap per compressed member and no cycle check outside strict mode. It reads the
player's own dump.
hookshot_log.txt includes full paths (user names).

A second pass after #1398. It checks that pass's fixes for ways around them, and covers the HLE services, the renderer's copies and draws, and the files players share. Each commit is one topic. ## Ways around the first pass ### fix(fs): read guest paths once, within bounds; more Windows device names - `host_path` copies the guest path once, then checks and uses the copy. Another guest thread could change the path between the check and the use. - `gstr` gives `""` for a path that does not end within 0x302 bytes of guest memory. - `guest_path_ok` also refuses `COM¹`-`COM³` and `LPT¹`-`LPT³` (UTF-8, the game's code page), `CLOCK$`, and a device name followed by spaces (`NUL .txt`). - `DeleteDirectoryRecursively` on a folder that is itself a link removes only the link. The posix layer's `GetFileAttributesA` marks links as reparse points, as Windows does. - **Tested:** `tests/test_fs_links.py`: new cases for the link attribute, and `guest_path_ok` built as Windows builds it (device names). ### fix(nvn): check the whole of each array the game passes - `P(i)` checks one byte. The arrays copied with a size now use `PN(i, n)`: border colours, viewports, vertex states, buffer ranges, render target lists, window textures, push constants, blend colour. ### fix(nvn): bound ReadTexels and WriteTexels to the game's buffer - The strided calls take 64-bit row and image strides from the game. Only the buffer's first byte was checked, and the renderer read or wrote at `data + k * img + y * row`: anywhere in the process. - The span the strides reach is now computed with overflow checks and must lie in guest memory. The texture, view and region objects are checked whole. - **Tested:** `tests/test_nvn_host_tables.py`: extents, strides that wrap, a stride past the window. ### fix(saves): also refuse Windows junctions on Python before 3.12 - `os.path.isjunction` exists from Python 3.12 only, and `copytree` walks into a junction as into a folder. A link is now any reparse point or symbolic link, from `os.lstat`. - **Tested:** `tests/test_oot_saves.py test_junction_refused` (a junction simulated through `os.lstat`). It fails on the old check. ### fix(env): hookshot_env.txt names no libraries, files to write or shares - SDL names that choose a library (`SDL_VULKAN_LIBRARY`, `SDL_VIDEO_EGL_DRIVER`, ...) are left out. A bare file name is looked for beside the game and in the current folder on Windows. - Names ending in `_FILE` or `_DIR`, `RX_VK_FRAME_LOG` and `HOS_PRESENT_TRACE` take no value from this file. A bare name resolves in the game folder: `HOS_SAMPLE_FILE=hookshot_vk.dll` emptied the renderer. - `RX_VK_DLSS_DIR` may not be a share (`\\server\x`, `//server/x`). `.` and `..` count as paths. - `HOS_ENV_FILE`'s file still sets anything. The readme says so. - **Tested:** `tests/test_env_file.py`, new lines for each case. ### fix(update): no file links in a downloaded Linux list; downloads read at most what they expect - `file://` part links are taken only from a list on this PC (the tools' own, `HOOKSHOT_LINUX_LIST`). In a downloaded list they could read `/dev/zero` until memory ran out, or test whether a local file has a given hash. - A download reads at most 4 GB. - **Tested:** `tests/test_linux_update.py`: the remote case and the limit. ## HLE services ### fix(video): check a decoder input's size and data - The submit size is 64-bit and was cut to an `int` for `av_new_packet`, then copied in whole: a heap overflow. A size past `INT_MAX` is refused, and the data must lie in guest memory. ### fix(audio): bound memory pool handles, microphone rings, reverb delays and Opus buffers - Memory pool handles are checked against the 1024-entry table. `ReleaseMemoryPool` wrote, and the getters read, past it. - An input stream holds at most 10 s. The ring's byte offsets are ints, and a large frame count overflowed them, which led to writes before the ring. - `ReadInputStream` writes only into guest memory. - I3DL2 reverb delays are clamped as doubles and from both sides. A negative late delay read before the delay line. - Opus decode: both buffers lie in guest memory, and the frame count is at most 120 ms. - **Tested:** `tests/test_audio_in.py`: a stereo stream of 0x20000001 frames. The old code crashes on it. ### fix(fs): refuse a mount when the mount table is full - A 17th mount name was written past `mounts[16]`. It now fails with a mount error. ### fix(nvdrv): keep channel sync point ids inside the table - A channel's sync point id grew with each GPFIFO allocation and indexed `syncpt[192]` unchecked. Ids are used again after the last one. ### fix(mem): check the guest ranges of heap and page calls - `SetMemoryHeapSize` and svc `SetHeapSize` check the size without wrap-around. A large size passed the old check, and a later shrink zeroed memory past the window. - `AllocateMemoryPages` and `FreeMemoryPages` refuse ranges outside the guest window. A wrapped range was zeroed whole. - `hos_commit` and `discard_pages` act on guest pages only. On Linux they change protection, or discard, whatever range they get. ### fix(os): message queue storage, calendar on bad times, vsync event slots - A message queue whose storage could not be allocated was written through `NULL`; it now stops with a message. - `ToCalendarTime` on a time the C library cannot convert returns zeros, not stack bytes. - `GetDisplayVsyncEvent` takes its slot under a lock. Two threads could write past the 8 slots. ### fix(windows): load XInput and IMM32 from System32 - `LOAD_LIBRARY_SEARCH_SYSTEM32`, as for `vulkan-1.dll`. ## Renderer ### fix(vk): bound copies, buffer binds and indirect draws by what the game names - The CPU copy from a buffer into a texture (ASTC) checks that its rows fit in what is mapped. It read past the mapping and past the guest window. - Copy row strides: the buffer size is computed with overflow checks, and a stride Vulkan cannot express is refused. Before, a wrapped size checked one row while Vulkan copied many. - Copy layer ranges are 64-bit. A range near 2^32 wrapped past the clamp. - `gpu_buffer_bind`'s fast path checks the size without wrap-around, as `pool_of` does. - Indirect draws bind the records at the stride they are read with. A negative count or stride is no draw. The mesh fallback draws only the records it converted. - Quad draws over 16M vertices are left out. The host buffers grew with the count, up to 16 GB. - Small: the CPU buffer clear writes at most its 256-byte pattern; packaged block heights and depths are at most 32 GOBs; `RX_VK_MESH_ARGS` no longer divides by zero. ### fix(vk): check the stage and slot of each kept texture answer - `hookshot_programs.bin` and the pipeline state list name a stage and a descriptor slot for each texture answer. They were used as indices unchecked, on every draw that found the record. - A cache record whose answers do not fit is left out and translated again. State list answers that do not fit are not used. - **Tested:** `tests/test_program_cache_cleanup.py`: an answer past the stages and an answer past the slots, both refused, their modules released. `tests/test_program_answer_domains.py` extracts the new helper. ## Other files players share ### fix(mods): do not walk a mod's romfs folder that is a link - Links to folders inside a mod were already left out. The romfs folder itself could be one (to `/`), and the index walked the whole disk. - **Tested:** `tests/test_fs_links.py`. ### fix(builder): refuse an update tag that names no folder of its own - The Windows builder cleans the tag into a folder name under `builders\` that the update replaces. `..` survived the cleaning. An empty name, a leading dot, or more than 128 characters is now refused, as the setup and the Linux lists do. - Not built here (no .NET on this machine); CI builds it. ### ci: keep the release token off redirects - `release_parts.get` sends `GH_TOKEN` with `add_unredirected_header`. urllib copies plain headers to a redirect, which may go to another host. ## How it was tested - `python -I -B tests/run_all.py`: 203 passed, 5 skipped (no tool for them here). - Linux build of the game and `hookshot_vk.so`: passes, and the vk ABI check is clean. - Windows: `hookshot_vk.dll` cross-built with llvm-mingw; a syntax check of every changed host file with llvm-mingw's clang. - Game run on Linux, in a headless compositor, from a copy of a save: - the game boots, loads the save and plays (walking, camera, a sword swing); - the Insert menu opens, moves and closes; - three runs, two of them first starts that prepared the shaders; - no error lines in the log. ## Not in this PR - `scripts/linux/deps.sh` downloads dav1d and the Vulkan, SPIR-V and VMA headers as tag archives without a hash (GitLab's are not byte-stable). Pinning them by commit archive or release tarball needs a decision. - The Linux release job uses `ubuntu:22.04` by tag and the toolchain PPA's `g++-13` unpinned; its libstdc++ is linked statically into the shipped renderer. - The program cache still passes its SPIR-V to the driver unvalidated. A cache from someone else is trusted once its stamp matches. An install key in the stamp, or a MAC, would close that. - `fonts/regular.ttf` and `bold.ttf` beside the game go to stb_truetype on Linux, which is not safe for untrusted fonts (a fuzzer finds out-of-bounds reads within a minute). The ImGui menu work replaces that path. - The Windows game itself does not call `SetDefaultDllDirectories`. Every load found here is now by full path or from System32, but SDL's own loads still use the default order. - `nx2/nup.py` extraction has no output cap per compressed member and no cycle check outside strict mode. It reads the player's own dump. - `hookshot_log.txt` includes full paths (user names).

Commenting is not possible because the repository is archived.

No reviewers