fix: harden what files, downloads and guest memory hand to host code #1398
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1398
Loading…
Reference in a new issue
No description provided.
Delete branch "security-hardening"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This PR checks inputs before host code uses them: files players pass around, release downloads, setup lists, and
guest memory. Each commit is one topic and can be reviewed alone.
Changes
fix(nvn): keep host pointers in host-side tables; bounds-check guest ranges
memory. They now live on the host side:
tests/test_nvn_host_tables.py(new): the real functions, covering window ranges including wrap-around, slots byowner and the live handle set;
tests/test_nvn_buffer_map.py, updated for the pool's guest address;ci: check release downloads and keep caches out of release builds
scripts/linux/deps.shpins the SHA-256 of the LLVM, Python, FFmpeg, SDL2 and appimagetool downloads and checksthem on every run. The Python one matches python-build-standalone's published SHA256SUMS.
unpacks them from their checked downloads. The Linux job's timeout goes to 60 minutes for that.
release_parts.py extractskips zip members that resolve outside the target folder.publish_parts.shwrites the release JSON to a private temporary file.fetch()with a matching and a wrong hash, by hand;bash -non both scripts.fix(builder): check setup list names; load the setup's libraries from System32
setup.candlinux_update.pyonly accept setup lists whose tag and part names are plain version names andrelative paths inside the install folder. Checksums must be 64 hex digits, and links must be https (also file://
on Linux, for local lists). The tag names a folder the update removes and remakes.
SetDefaultDllDirectories(System32)and is linked with--dependent-load-flag 0x800, so the libraries it imports come from System32, not from the folder it wasdownloaded to.
Runner.csuses a source tree only when the app sits in that tree's builder folder.tests/test_linux_update.py: a new case with bad tags, paths, checksums and links;setup.ccross-built with llvm-mingw;llvm-readobj --coff-load-configshowsDependentLoadFlags: 0x800;fix(vk): check the shaders seed, program cache, pipeline notes and state list as read
hookshot_shaders.bin(seed_open): every count, size and stage index is checked against the file.hookshot_programs.bin: record sizes are compared unsigned, so a size past 2^63 no longer makes the scan gobackwards.
hookshot_pipeline_states.binandhookshot_pipelines.bin:build_graphics,gpl_linkandpipeline_keyalso refuse more than 8 targets.mkw_vk_pipelines.bin: the header's length and version are checked too.tests/test_seed_reader.py(new): the real reader under ASan/UBSan, on cut, oversized and mutated files(20,000 mutations). It fails on the old reader;
tests/test_pipeline_state_counts.pyandtests/test_program_cache_index.py.fix(saves): refuse links in a save being imported
scripts/oot_saves.pycopied a save withcopytree/copy2, which follow links. A link in a save fromanother program now stops the copy before anything changes, and the copy no longer follows links.
tests/test_oot_saves.py test_links_in_a_save_refused(a linked.sav, a linkedaccount.sav, afolder link).
fix(fs): keep guest paths in their mounts; do not work through links
host_pathmaps no guest path with a.or..part, a\or:, or a control character. On Windows it alsorefuses a part that ends in a dot or a space, and device names. It does not use a host path that was cut short.
DeleteDirectoryRecursivelyremoves a link inside the folder itself, never what it points to.managers place files that way.
tests/test_fs_links.py(new; the real functions over the posix layer).fix(env): hookshot_env.txt sets only the game's own switches
RX_,HOS_,HOOKSHOT_,EDEN_,NVN_,TOTK_orSDL_. It never setsHOS_ENV_FILEorSDL_DYNAMIC_API, and takes no path as a value exceptRX_VK_DLSS_DIR's. Each line it leaves out is logged. A file named byHOS_ENV_FILE, as test runs use, still setsanything. The readme says so.
how the system's libraries behave, and some switches name paths.
tests/test_env_file.py(the player's file andHOS_ENV_FILE's).fix(update): check the latest release's tag on Windows too
. _ + -) before the tag goes on theBuilder's command line.
fix(windows): load the Vulkan loader and SDL from known folders only
vulkan-1.dllis loaded from System32. SDL, when it is not beside the executable, is found without thecurrent folder or PATH.
hookshot_vk.dllcross-built with llvm-mingw.How it was tested
python -I -B tests/run_all.py: 200 passed, 7 skipped (no tool for them here).nx2/test_nup_read_name.pyneedszstandard; it passes with the bundled Python, as dotest_nup_codec.pyandtest_nup_verify.py.hookshot_vk.so: a full build passes (hookshotandhookshot_vk.so), and the vk ABI check is clean.hookshot_vk.dll, the setup exe, and a syntax check of every changed host file.hookshot_env.txtwithVK_LOADER_DEBUGandHOS_SAMPLE_FILElines logged both as left out.Not in this PR
npm ciinstead ofnpm install).These need a decision from the maintainer first.