From 5f0d1be078e67f33d3eaa9041bac10bd466b6636 Mon Sep 17 00:00:00 2001 From: Knutwurst <36196269+knutwurst@users.noreply.github.com> Date: Tue, 23 Jun 2026 19:46:23 +0200 Subject: [PATCH] Harden filesystem safety after a security review Defense in depth around the only operations that touch the filesystem, so no request can escape /data/patchdl or leave the process able to write to a system path: - Validate the HTTP title_id with path_segment_safe at the top of the title-action route, and again inside remove_title_dir and cleanup_installed_download. The delete primitives are now self- protecting instead of relying only on the "title exists in the scan" guard, so a future refactor cannot reintroduce a /data-wiping traversal (a title_id of ".." would otherwise resolve the dir to /data). - Only swap the process root vnode when the current root was captured and can be restored, in both the scan and the debug dump. Otherwise the process could be left rooted at the system root, sending later absolute-path writes to the wrong place. Reviewed and confirmed safe with no change needed: the installer only delegates to Sony's signed AppInstUtil service (it never writes or redirects to system paths itself), app.db is opened read-only and immutable, every write targets /data/patchdl, the patch picker never selects an update that needs a newer firmware, and the /api/pkg file server already blocks path traversal. --- src/patchdl_scan.c | 16 ++++++++++++---- src/patchdl_websrv.c | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/patchdl_scan.c b/src/patchdl_scan.c index 0d1e40b..496b436 100644 --- a/src/patchdl_scan.c +++ b/src/patchdl_scan.c @@ -415,8 +415,13 @@ patchdl_scan(patchdl_title_t **titles_out, size_t *count_out) { root_vnode = kernel_get_root_vnode(); if (root_vnode) { saved_root = kernel_get_proc_rootdir(pid); - kernel_set_proc_rootdir(pid, root_vnode); - using_vswap = 1; + /* Only swap if we captured the current root, so we can always restore + it. Leaving the process rooted at the system root would make later + absolute-path writes land in the wrong place. */ + if (saved_root) { + kernel_set_proc_rootdir(pid, root_vnode); + using_vswap = 1; + } } for (int i = 0; SCAN_DIRS[i]; i++) @@ -486,8 +491,11 @@ patchdl_scan_debug_json(void) { root_vnode = kernel_get_root_vnode(); if (root_vnode) { saved_root = kernel_get_proc_rootdir(pid); - kernel_set_proc_rootdir(pid, root_vnode); - using_vswap = 1; + /* Only swap if we can restore it afterwards (see patchdl_scan). */ + if (saved_root) { + kernel_set_proc_rootdir(pid, root_vnode); + using_vswap = 1; + } } for (int i = 0; SCAN_DIRS[i]; i++) { diff --git a/src/patchdl_websrv.c b/src/patchdl_websrv.c index 7f867f9..5e4fa2b 100644 --- a/src/patchdl_websrv.c +++ b/src/patchdl_websrv.c @@ -644,6 +644,8 @@ static void cleanup_installed_download(const char *title_id, const char *patch_url) { char dir[256], path[320]; const char *base = strrchr(patch_url, '/'); + if (!path_segment_safe(title_id)) + return; base = base ? base + 1 : "patch.pkg"; snprintf(dir, sizeof(dir), "/data/patchdl/%s", title_id); snprintf(path, sizeof(path), "%s/%s", dir, base); @@ -800,6 +802,12 @@ remove_title_dir(const char *title_id) { DIR *d; struct dirent *e; + /* Self-protecting: never build a delete path from an unsafe segment, even + if a future caller forgets the upstream check. A title_id of ".." would + otherwise resolve dir to /data and wipe it. */ + if (!path_segment_safe(title_id)) + return; + snprintf(dir, sizeof(dir), "%s/%s", PATCHDL_DL_DIR, title_id); if ((d = opendir(dir))) { while ((e = readdir(d))) { @@ -1006,6 +1014,14 @@ handle_title_action(struct MHD_Connection *conn, const char *url) { action, sizeof(action))) return queue_text(conn, MHD_HTTP_NOT_FOUND, "not found"); + /* Defense in depth: title_id becomes part of /data/patchdl/ paths that + get created, written, and recursively deleted. Reject anything that + isn't a plain id BEFORE it can reach mkdir/unlink/rmdir, so no request + can ever escape the download directory (a title_id of ".." would resolve + the dir to /data). Real PS5 title ids only use [A-Za-z0-9_-.]. */ + if (!path_segment_safe(title_id)) + return queue_text(conn, MHD_HTTP_FORBIDDEN, "forbidden"); + if (!get_title_action_info(title_id, &src, patch_url, sizeof(patch_url), patch_title_id, sizeof(patch_title_id), patch_storage_title_id,