From 785bf37b4a277dc7a06ae59eccee1cb5e700a5d8 Mon Sep 17 00:00:00 2001
From: mleem97 <52848568+mleem97@users.noreply.github.com>
Date: Wed, 29 Jul 2026 00:20:08 +0000
Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?=
 =?UTF-8?q?=20Fix=20Path=20Traversal=20in=20SetPortrait?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .jules/sentinel.md               |  4 ++++
 src/API/CustomEmployeeManager.cs | 13 +++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 157d3ed8..e6968c2f 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-07-28 - Path Traversal in CustomEmployeeManager.SetPortrait
+**Vulnerability:** The `SetPortrait` method in `src/API/CustomEmployeeManager.cs` concatenated `employeeId` directly into `Path.Combine` without validating it. This could allow for path traversal attacks if a malicious `employeeId` bypassed the initial `Register` check or was loaded through a vulnerable API call.
+**Learning:** Even if an identifier is sanitized at registration, secondary methods that accept it as input (especially those interacting with the file system) must independently validate it to enforce defense in depth.
+**Prevention:** Validate input strings forming file paths immediately before using `Path.Combine`. Reject inputs containing traversal sequences like `..` or characters from `Path.GetInvalidFileNameChars()`.
diff --git a/src/API/CustomEmployeeManager.cs b/src/API/CustomEmployeeManager.cs
index 9b4512b6..1459485c 100644
--- a/src/API/CustomEmployeeManager.cs
+++ b/src/API/CustomEmployeeManager.cs
@@ -889,12 +889,17 @@ private static void SetPortrait(Transform card, string employeeId)
             var portraitTransform = card.Find("Image");
             if (portraitTransform == null) return;
 
-            string assetsDir = Path.Combine(MelonEnvironment.UserDataDirectory, "ModAssets");
             string? imagePath = null;
-            foreach (var ext in new[] { ".jpg", ".png" })
+
+            // Security: Prevent path traversal by ensuring the employeeId does not contain directory separators or invalid characters.
+            if (!string.IsNullOrEmpty(employeeId) && employeeId.IndexOfAny(Path.GetInvalidFileNameChars()) < 0 && !employeeId.Contains(".."))
             {
-                string candidate = Path.Combine(assetsDir, employeeId + ext);
-                if (File.Exists(candidate)) { imagePath = candidate; break; }
+                string assetsDir = Path.Combine(MelonEnvironment.UserDataDirectory, "ModAssets");
+                foreach (var ext in new[] { ".jpg", ".png" })
+                {
+                    string candidate = Path.Combine(assetsDir, employeeId + ext);
+                    if (File.Exists(candidate)) { imagePath = candidate; break; }
+                }
             }
 
             if (imagePath != null)