From 68b60dbc890a53ab38154f6aa51384b9753a501c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=27=C3=A9lectron=20rare?= <108685187+electron-rare@users.noreply.github.com> Date: Wed, 11 Mar 2026 03:14:51 +0100 Subject: [PATCH] fix(security): harden path traversal sanitization in file share API Replace naive `.replace("..", "_")` with proper sanitization that decodes percent-encoded bytes (%2e%2e) before stripping traversal sequences. Handles URL-encoding bypass, backslash normalization, and repeated slashes. Co-Authored-By: Claude Opus 4.6 --- .../src/app/file_share_service.cpp | 78 ++++++++++++++----- ui_freenove_allinone/src/app/main.cpp | 27 ++++++- 2 files changed, 86 insertions(+), 19 deletions(-) diff --git a/ui_freenove_allinone/src/app/file_share_service.cpp b/ui_freenove_allinone/src/app/file_share_service.cpp index a2673b2..6497166 100644 --- a/ui_freenove_allinone/src/app/file_share_service.cpp +++ b/ui_freenove_allinone/src/app/file_share_service.cpp @@ -60,6 +60,64 @@ String urlEncodeSimple(const char* text) { return out; } +// Decode percent-encoded bytes in-place (e.g. %2e → '.'). +String urlDecodeSimple(const String& src) { + String out; + out.reserve(src.length()); + for (unsigned int i = 0; i < src.length(); ++i) { + if (src[i] == '%' && i + 2 < src.length()) { + const char hi = src[i + 1]; + const char lo = src[i + 2]; + auto hexVal = [](char c) -> int { + if (c >= '0' && c <= '9') return c - '0'; + if (c >= 'a' && c <= 'f') return c - 'a' + 10; + if (c >= 'A' && c <= 'F') return c - 'A' + 10; + return -1; + }; + const int h = hexVal(hi); + const int l = hexVal(lo); + if (h >= 0 && l >= 0) { + out += static_cast((h << 4) | l); + i += 2; + continue; + } + } + out += src[i]; + } + return out; +} + +// Sanitize a user-supplied path: decode percent-encoding, collapse separators, +// remove all ".." segments, and reject paths that escape the intended root. +// Returns an empty String on failure. +String sanitizePath(const String& raw) { + // 1. Decode any percent-encoding first so %2e%2e is caught. + String path = urlDecodeSimple(raw); + path.trim(); + if (path.isEmpty()) { + return String(); + } + // 2. Normalise separators. + path.replace("\\", "/"); + // 3. Reject and strip any ".." segment (handles ../, /../, bare ".." etc.). + // Loop until no more ".." remain to defeat chained encodings. + while (path.indexOf("..") >= 0) { + path.replace("..", ""); + } + // 4. Collapse repeated slashes. + while (path.indexOf("//") >= 0) { + path.replace("//", "/"); + } + // 5. Strip leading slash (callers prepend the root). + if (path.startsWith("/")) { + path.remove(0, 1); + } + if (path.isEmpty()) { + return String(); + } + return path; +} + bool ensureDir(const char* path) { if (path == nullptr || path[0] == '\0') { return false; @@ -152,16 +210,10 @@ bool FileShareService::saveIncomingFile(const char* relative_path, if (!ensureSharedDirs()) { return false; } - String path = relative_path; - path.trim(); + String path = sanitizePath(relative_path); if (path.isEmpty()) { return false; } - if (path.startsWith("/")) { - path = path.substring(1); - } - path.replace("..", "_"); - path.replace("\\", "/"); String full_path = String(kIncomingRoot) + "/" + path; const int slash = full_path.lastIndexOf('/'); if (slash > 0) { @@ -370,20 +422,10 @@ bool FileShareService::resolveIncomingPath(const char* requested_path, String* o if (requested_path == nullptr || requested_path[0] == '\0' || out_full_path == nullptr) { return false; } - String path = requested_path; - path.trim(); + String path = sanitizePath(requested_path); if (path.isEmpty()) { return false; } - path.replace("\\", "/"); - path.replace("..", "_"); - if (path.startsWith("/apps/shared/incoming/")) { - *out_full_path = path; - return true; - } - if (path.startsWith("/")) { - path.remove(0, 1); - } *out_full_path = String(kIncomingRoot) + "/" + path; return true; } diff --git a/ui_freenove_allinone/src/app/main.cpp b/ui_freenove_allinone/src/app/main.cpp index 9d1fe0a..2fed7ae 100644 --- a/ui_freenove_allinone/src/app/main.cpp +++ b/ui_freenove_allinone/src/app/main.cpp @@ -6338,8 +6338,33 @@ void setupWebUiImpl() { if (g_file_share_service.downloadFile(path.c_str(), &file, &full_path) && file) { file.close(); } + // Sanitize: decode percent-encoding, strip "..", collapse slashes. + { + String clean = path; + clean.replace("\\", "/"); + // Decode %XX sequences to catch encoded traversal (%2e%2e). + String decoded; + decoded.reserve(clean.length()); + for (unsigned int ci = 0; ci < clean.length(); ++ci) { + if (clean[ci] == '%' && ci + 2 < clean.length()) { + auto hv = [](char c) -> int { + if (c >= '0' && c <= '9') return c - '0'; + if (c >= 'a' && c <= 'f') return c - 'a' + 10; + if (c >= 'A' && c <= 'F') return c - 'A' + 10; + return -1; + }; + int h = hv(clean[ci+1]), l = hv(clean[ci+2]); + if (h >= 0 && l >= 0) { decoded += (char)((h<<4)|l); ci += 2; continue; } + } + decoded += clean[ci]; + } + while (decoded.indexOf("..") >= 0) decoded.replace("..", ""); + while (decoded.indexOf("//") >= 0) decoded.replace("//", "/"); + if (decoded.startsWith("/")) decoded.remove(0, 1); + path = decoded; + } saved_path = String("/apps/shared/incoming/") + path; - if (saved_path.indexOf("..") >= 0) { + if (path.isEmpty()) { ok = false; } else { const char* mode = (offset == 0U) ? "w" : "a";