🛡️ Sentinel: [CRITICAL] Fix prefix-matching path traversal in Lua IO sandbox

.jules/sentinel.md

@@ -11,3 +11,7 @@
 **Vulnerability:** Path traversal vulnerability due to unsanitized `modId` in `GetConfigPath` in `src/Compatibility/DataCenterModLoader/ModConfigSystem.cs`.
 **Learning:** Concatenating user input (like a `modId`) directly into `Path.Combine` allows for directory traversal attacks (`../`, etc.) leading to arbitrary file read/write issues.
 **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-20 - Prevent Prefix-Matching Path Traversal Bypass in Sandbox Validation
+**Vulnerability:** A prefix-matching path traversal vulnerability was found in `GregIoLuaModule.cs`. A path like `/mods/Lua/my_mod/data_secret` would bypass the `StartsWith` sandbox boundary check for the base directory `/mods/Lua/my_mod/data` because it literally starts with the same string but lacks the directory separator constraint.
+**Learning:** `String.StartsWith` on file paths without guaranteeing a trailing slash on the base directory is insufficient for sandbox enforcement and allows traversing to sibling directories that share the same prefix.
+**Prevention:** Always ensure the base directory string ends with `Path.DirectorySeparatorChar` before performing a `String.StartsWith` check, and allow exact matches to the base directory itself.

src/Infrastructure/Scripting/Lua/Modules/GregIoLuaModule.cs

@@ -1,148 +1,155 @@
-/// <file-summary>
-/// Schicht:      Infrastructure
-/// Zweck:        Sandboxed IO Funktionen für Lua.
-/// Maintainer:   Darf nur auf {modDir}/data/ zugreifen.
-///               greg.io.read_file(), write_file(), file_exists(), list_files(), delete_file()
-/// </file-summary>
-
-using System;
-using System.IO;
-using System.Linq;
-using MoonSharp.Interpreter;
-using MelonLoader;
-
-namespace gregCore.Infrastructure.Scripting.Lua.Modules;
-
-public static class GregIoLuaModule
-{
-    /// <summary>
-    /// Registriert sandboxed I/O-Funktionen im greg.io Table.
-    /// </summary>
-    public static void Register(Table greg, Script script, string modId, string modDir)
-    {
-        string dataDir = Path.Combine(modDir, "data");
-        Directory.CreateDirectory(dataDir);
-
-        var ioTable = new Table(script);
-
-        // greg.io.read_file(path) → string
-        ioTable["read_file"] = (Func<string, string>)(path =>
-        {
-            try
-            {
-                string fullPath = ResolveSafe(dataDir, path);
-                return File.ReadAllText(fullPath);
-            }
-            catch (Exception ex)
-            {
-                MelonLogger.Error($"[LuaMod:{modId}] io.read_file('{path}') failed: {ex.Message}");
-                return "";
-            }
-        });
-
-        // greg.io.write_file(path, content)
-        ioTable["write_file"] = (Action<string, string>)((path, content) =>
-        {
-            try
-            {
-                string fullPath = ResolveSafe(dataDir, path);
-                string dir = Path.GetDirectoryName(fullPath)!;
-                if (!Directory.Exists(dir)) Directory.CreateDirectory(dir);
-                File.WriteAllText(fullPath, content);
-            }
-            catch (Exception ex)
-            {
-                MelonLogger.Error($"[LuaMod:{modId}] io.write_file('{path}') failed: {ex.Message}");
-            }
-        });
-
-        // greg.io.append_file(path, content)
-        ioTable["append_file"] = (Action<string, string>)((path, content) =>
-        {
-            try
-            {
-                string fullPath = ResolveSafe(dataDir, path);
-                File.AppendAllText(fullPath, content);
-            }
-            catch (Exception ex)
-            {
-                MelonLogger.Error($"[LuaMod:{modId}] io.append_file('{path}') failed: {ex.Message}");
-            }
-        });
-
-        // greg.io.file_exists(path) → bool
-        ioTable["file_exists"] = (Func<string, bool>)(path =>
-        {
-            try
-            {
-                string fullPath = ResolveSafe(dataDir, path);
-                return File.Exists(fullPath);
-            }
-            catch
-            {
-                return false;
-            }
-        });
-
-        // greg.io.delete_file(path)
-        ioTable["delete_file"] = (Action<string>)(path =>
-        {
-            try
-            {
-                string fullPath = ResolveSafe(dataDir, path);
-                if (File.Exists(fullPath)) File.Delete(fullPath);
-            }
-            catch (Exception ex)
-            {
-                MelonLogger.Error($"[LuaMod:{modId}] io.delete_file('{path}') failed: {ex.Message}");
-            }
-        });
-
-        // greg.io.list_files(pattern?) → table of strings
-        ioTable["list_files"] = (Func<string?, Table>)(pattern =>
-        {
-            try
-            {
-                var files = Directory.GetFiles(dataDir, pattern ?? "*.*", SearchOption.AllDirectories)
-                    .Select(f => Path.GetRelativePath(dataDir, f).Replace('\\', '/'))
-                    .ToArray();
-
-                var table = new Table(script);
-                for (int i = 0; i < files.Length; i++)
-                {
-                    table[i + 1] = files[i]; // Lua arrays are 1-indexed
-                }
-                return table;
-            }
-            catch (Exception ex)
-            {
-                MelonLogger.Error($"[LuaMod:{modId}] io.list_files failed: {ex.Message}");
-                return new Table(script);
-            }
-        });
-
-        // greg.io.data_dir → string (read-only)
-        ioTable["data_dir"] = dataDir.Replace('\\', '/');
-
-        greg["io"] = ioTable;
-    }
-
-    /// <summary>
-    /// Löst einen relativen Pfad auf und validiert, dass er innerhalb des Data-Dirs liegt.
-    /// </summary>
-    private static string ResolveSafe(string dataDir, string relativePath)
-    {
-        if (string.IsNullOrWhiteSpace(relativePath))
-            throw new InvalidOperationException("Path cannot be empty");
-
-        // Prevent path traversal
-        string normalized = relativePath.Replace('/', Path.DirectorySeparatorChar);
-        string fullPath = Path.GetFullPath(Path.Combine(dataDir, normalized));
-        string dataDirFull = Path.GetFullPath(dataDir);
-
-        if (!fullPath.StartsWith(dataDirFull, StringComparison.OrdinalIgnoreCase))
-            throw new UnauthorizedAccessException($"Access denied: path escapes sandbox ('{relativePath}')");
-
-        return fullPath;
-    }
-}
+/// <file-summary>
+/// Schicht:      Infrastructure
+/// Zweck:        Sandboxed IO Funktionen für Lua.
+/// Maintainer:   Darf nur auf {modDir}/data/ zugreifen.
+///               greg.io.read_file(), write_file(), file_exists(), list_files(), delete_file()
+/// </file-summary>
+
+using System;
+using System.IO;
+using System.Linq;
+using MoonSharp.Interpreter;
+using MelonLoader;
+
+namespace gregCore.Infrastructure.Scripting.Lua.Modules;
+
+public static class GregIoLuaModule
+{
+    /// <summary>
+    /// Registriert sandboxed I/O-Funktionen im greg.io Table.
+    /// </summary>
+    public static void Register(Table greg, Script script, string modId, string modDir)
+    {
+        string dataDir = Path.Combine(modDir, "data");
+        Directory.CreateDirectory(dataDir);
+
+        var ioTable = new Table(script);
+
+        // greg.io.read_file(path) → string
+        ioTable["read_file"] = (Func<string, string>)(path =>
+        {
+            try
+            {
+                string fullPath = ResolveSafe(dataDir, path);
+                return File.ReadAllText(fullPath);
+            }
+            catch (Exception ex)
+            {
+                MelonLogger.Error($"[LuaMod:{modId}] io.read_file('{path}') failed: {ex.Message}");
+                return "";
+            }
+        });
+
+        // greg.io.write_file(path, content)
+        ioTable["write_file"] = (Action<string, string>)((path, content) =>
+        {
+            try
+            {
+                string fullPath = ResolveSafe(dataDir, path);
+                string dir = Path.GetDirectoryName(fullPath)!;
+                if (!Directory.Exists(dir)) Directory.CreateDirectory(dir);
+                File.WriteAllText(fullPath, content);
+            }
+            catch (Exception ex)
+            {
+                MelonLogger.Error($"[LuaMod:{modId}] io.write_file('{path}') failed: {ex.Message}");
+            }
+        });
+
+        // greg.io.append_file(path, content)
+        ioTable["append_file"] = (Action<string, string>)((path, content) =>
+        {
+            try
+            {
+                string fullPath = ResolveSafe(dataDir, path);
+                File.AppendAllText(fullPath, content);
+            }
+            catch (Exception ex)
+            {
+                MelonLogger.Error($"[LuaMod:{modId}] io.append_file('{path}') failed: {ex.Message}");
+            }
+        });
+
+        // greg.io.file_exists(path) → bool
+        ioTable["file_exists"] = (Func<string, bool>)(path =>
+        {
+            try
+            {
+                string fullPath = ResolveSafe(dataDir, path);
+                return File.Exists(fullPath);
+            }
+            catch
+            {
+                return false;
+            }
+        });
+
+        // greg.io.delete_file(path)
+        ioTable["delete_file"] = (Action<string>)(path =>
+        {
+            try
+            {
+                string fullPath = ResolveSafe(dataDir, path);
+                if (File.Exists(fullPath)) File.Delete(fullPath);
+            }
+            catch (Exception ex)
+            {
+                MelonLogger.Error($"[LuaMod:{modId}] io.delete_file('{path}') failed: {ex.Message}");
+            }
+        });
+
+        // greg.io.list_files(pattern?) → table of strings
+        ioTable["list_files"] = (Func<string?, Table>)(pattern =>
+        {
+            try
+            {
+                var files = Directory.GetFiles(dataDir, pattern ?? "*.*", SearchOption.AllDirectories)
+                    .Select(f => Path.GetRelativePath(dataDir, f).Replace('\\', '/'))
+                    .ToArray();
+
+                var table = new Table(script);
+                for (int i = 0; i < files.Length; i++)
+                {
+                    table[i + 1] = files[i]; // Lua arrays are 1-indexed
+                }
+                return table;
+            }
+            catch (Exception ex)
+            {
+                MelonLogger.Error($"[LuaMod:{modId}] io.list_files failed: {ex.Message}");
+                return new Table(script);
+            }
+        });
+
+        // greg.io.data_dir → string (read-only)
+        ioTable["data_dir"] = dataDir.Replace('\\', '/');
+
+        greg["io"] = ioTable;
+    }
+
+    /// <summary>
+    /// Löst einen relativen Pfad auf und validiert, dass er innerhalb des Data-Dirs liegt.
+    /// </summary>
+    private static string ResolveSafe(string dataDir, string relativePath)
+    {
+        if (string.IsNullOrWhiteSpace(relativePath))
+            throw new InvalidOperationException("Path cannot be empty");
+
+        // Prevent path traversal
+        string normalized = relativePath.Replace('/', Path.DirectorySeparatorChar);
+        string fullPath = Path.GetFullPath(Path.Combine(dataDir, normalized));
+        string dataDirFull = Path.GetFullPath(dataDir);
+
+        string dataDirWithSep = dataDirFull;
+        if (!dataDirWithSep.EndsWith(Path.DirectorySeparatorChar.ToString()))
+            dataDirWithSep += Path.DirectorySeparatorChar;
+
+        if (!fullPath.Equals(dataDirFull, StringComparison.OrdinalIgnoreCase) &&
+            !fullPath.StartsWith(dataDirWithSep, StringComparison.OrdinalIgnoreCase))
+        {
+            throw new UnauthorizedAccessException($"Access denied: path escapes sandbox ('{relativePath}')");
+        }
+
+        return fullPath;
+    }
+}