From 04ad286948fa3bd5657f15d1d1457d858d1db854 Mon Sep 17 00:00:00 2001 From: mleem97 <52848568+mleem97@users.noreply.github.com> Date: Mon, 3 Aug 2026 00:11:02 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]=20Fi?= =?UTF-8?q?x=20path=20traversal=20in=20SetPortrait?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🚨 Severity: HIGH 💡 Vulnerability: A path traversal vulnerability was present in `CustomEmployeeManager.SetPortrait` where unvalidated `employeeId` strings from external mods could be used in file path construction. 🎯 Impact: Allowed malicious mods to access arbitrary files on the user's system by crafting paths using `../` in the `employeeId` parameter when attempting to load portraits. 🔧 Fix: Added independent path traversal validation `IndexOfAny(Path.GetInvalidFileNameChars()) < 0 && !employeeId.Contains("..")` for the `employeeId` before constructing the file path, gracefully falling back to a default solid teal color if validation fails. ✅ Verification: Run `dotnet test` to verify the codebase functions as expected and verify that the `SetPortrait` method safely rejects identifiers containing path navigation characters. --- .jules/sentinel.md | 4 ++++ src/API/CustomEmployeeManager.cs | 17 ++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 157d3ed8..92578335 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -21,3 +21,7 @@ **Vulnerability:** `CustomEmployeeManager.Register` accepted arbitrary employee IDs without validation, which were later used directly in `Path.Combine` to construct image loading paths, enabling path traversal (CWE-22). **Learning:** Identifiers provided by mods or external sources must be treated as untrusted input and validated before being used in file system operations. **Prevention:** Validate input strings that form part of a file path before concatenating them. Reject them if they contain directory traversal characters like `..`, `Path.DirectorySeparatorChar`, `Path.AltDirectorySeparatorChar`, or any invalid filename characters (using `Path.GetInvalidFileNameChars()`). +## 2024-05-24 - Prevent Path Traversal via Arbitrary Identifiers +**Vulnerability:** A path traversal vulnerability existed in `CustomEmployeeManager.SetPortrait` because arbitrary `employeeId` strings from mods were used in `Path.Combine` without being validated. +**Learning:** Even when reading benign assets (like portraits), any string identifier that originates externally must be validated before being used to construct a file path. +**Prevention:** Use `id.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0 || id.Contains("..")` to validate these external identifiers rather than relying on directory or environment constraints alone. diff --git a/src/API/CustomEmployeeManager.cs b/src/API/CustomEmployeeManager.cs index 9b4512b6..a2d90821 100644 --- a/src/API/CustomEmployeeManager.cs +++ b/src/API/CustomEmployeeManager.cs @@ -891,10 +891,21 @@ private static void SetPortrait(Transform card, string employeeId) string assetsDir = Path.Combine(MelonEnvironment.UserDataDirectory, "ModAssets"); string? imagePath = null; - foreach (var ext in new[] { ".jpg", ".png" }) + + // Prevent path traversal by validating the arbitrary employeeId string + bool isSafePath = employeeId.IndexOfAny(Path.GetInvalidFileNameChars()) < 0 && !employeeId.Contains(".."); + + if (isSafePath) + { + foreach (var ext in new[] { ".jpg", ".png" }) + { + string candidate = Path.Combine(assetsDir, employeeId + ext); + if (File.Exists(candidate)) { imagePath = candidate; break; } + } + } + else { - string candidate = Path.Combine(assetsDir, employeeId + ext); - if (File.Exists(candidate)) { imagePath = candidate; break; } + CrashLog.Log($"[Security] Prevented path traversal attempt in SetPortrait for employeeId: {employeeId}"); } if (imagePath != null)