launcher: reject manifest paths that escape the client root
Defence-in-depth against a buggy or crafted manifest that points outside the client directory. Path resolution is centralised in PathSafety.ResolveInside so the orchestration code cannot forget the check; the applier still sees already-validated absolute paths. Rejected cases: parent traversal, mixed traversal, absolute paths, sibling-root lexical escapes. Required files that fail the check abort the update; optional files are skipped and logged.
This commit is contained in:
52
src/Metin2Launcher/Apply/PathSafety.cs
Normal file
52
src/Metin2Launcher/Apply/PathSafety.cs
Normal file
@@ -0,0 +1,52 @@
|
||||
namespace Metin2Launcher.Apply;
|
||||
|
||||
/// <summary>
|
||||
/// Guards against manifest-supplied paths escaping the client root.
|
||||
///
|
||||
/// Every path in a manifest is a relative path under the client directory. A malicious
|
||||
/// or buggy manifest that contains <c>..</c> segments, absolute paths, or drive letters
|
||||
/// could otherwise trick the applier into writing outside the client root
|
||||
/// (e.g. <c>../../../etc/passwd</c> or <c>C:\Windows\System32\evil.dll</c>). The
|
||||
/// signature verifier is our primary defence — an attacker cannot produce a valid
|
||||
/// manifest without the private key — but defence-in-depth demands this check too.
|
||||
/// </summary>
|
||||
public static class PathSafety
|
||||
{
|
||||
/// <summary>
|
||||
/// Resolves <paramref name="relativePath"/> under <paramref name="clientRoot"/> and
|
||||
/// confirms the result stays inside the root. Returns the absolute final path.
|
||||
/// Throws <see cref="UnsafePathException"/> if the path escapes.
|
||||
/// </summary>
|
||||
public static string ResolveInside(string clientRoot, string relativePath)
|
||||
{
|
||||
if (string.IsNullOrEmpty(relativePath))
|
||||
throw new UnsafePathException("empty manifest path");
|
||||
|
||||
if (Path.IsPathRooted(relativePath))
|
||||
throw new UnsafePathException($"rooted manifest path not allowed: {relativePath}");
|
||||
|
||||
// Normalise to the native separator so Path.GetFullPath handles segments uniformly.
|
||||
var localized = relativePath.Replace('/', Path.DirectorySeparatorChar);
|
||||
|
||||
var rootFull = Path.GetFullPath(clientRoot);
|
||||
var candidate = Path.GetFullPath(Path.Combine(rootFull, localized));
|
||||
|
||||
var rootWithSep = rootFull.EndsWith(Path.DirectorySeparatorChar)
|
||||
? rootFull
|
||||
: rootFull + Path.DirectorySeparatorChar;
|
||||
|
||||
if (candidate.Length < rootWithSep.Length ||
|
||||
!candidate.StartsWith(rootWithSep, StringComparison.Ordinal))
|
||||
{
|
||||
throw new UnsafePathException(
|
||||
$"manifest path escapes client root: {relativePath} -> {candidate}");
|
||||
}
|
||||
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
|
||||
public sealed class UnsafePathException : Exception
|
||||
{
|
||||
public UnsafePathException(string message) : base(message) { }
|
||||
}
|
||||
1
tests/Metin2Launcher.Tests/GlobalUsings.cs
Normal file
1
tests/Metin2Launcher.Tests/GlobalUsings.cs
Normal file
@@ -0,0 +1 @@
|
||||
global using Xunit;
|
||||
75
tests/Metin2Launcher.Tests/PathSafetyTests.cs
Normal file
75
tests/Metin2Launcher.Tests/PathSafetyTests.cs
Normal file
@@ -0,0 +1,75 @@
|
||||
using Metin2Launcher.Apply;
|
||||
using Xunit;
|
||||
|
||||
namespace Metin2Launcher.Tests;
|
||||
|
||||
public class PathSafetyTests : IDisposable
|
||||
{
|
||||
private readonly string _root;
|
||||
|
||||
public PathSafetyTests()
|
||||
{
|
||||
_root = Path.Combine(Path.GetTempPath(), "metin-pathsafe-" + Guid.NewGuid());
|
||||
Directory.CreateDirectory(_root);
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
try { Directory.Delete(_root, recursive: true); } catch { }
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Resolve_allows_simple_relative_path()
|
||||
{
|
||||
var result = PathSafety.ResolveInside(_root, "pack/item.pck");
|
||||
Assert.StartsWith(Path.GetFullPath(_root), result);
|
||||
Assert.EndsWith("item.pck", result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Resolve_allows_nested_relative_path()
|
||||
{
|
||||
var result = PathSafety.ResolveInside(_root, "assets/root/serverinfo.py");
|
||||
Assert.StartsWith(Path.GetFullPath(_root), result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Resolve_rejects_parent_traversal()
|
||||
{
|
||||
Assert.Throws<UnsafePathException>(
|
||||
() => PathSafety.ResolveInside(_root, "../../../etc/passwd"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Resolve_rejects_mixed_traversal()
|
||||
{
|
||||
Assert.Throws<UnsafePathException>(
|
||||
() => PathSafety.ResolveInside(_root, "assets/../../outside.txt"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Resolve_rejects_absolute_unix_path()
|
||||
{
|
||||
Assert.Throws<UnsafePathException>(
|
||||
() => PathSafety.ResolveInside(_root, "/etc/passwd"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Resolve_rejects_empty()
|
||||
{
|
||||
Assert.Throws<UnsafePathException>(
|
||||
() => PathSafety.ResolveInside(_root, ""));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Resolve_rejects_sibling_root_escape()
|
||||
{
|
||||
// If client root is /tmp/x and attacker passes ../x2/a.txt, the resolved path
|
||||
// becomes /tmp/x2/a.txt which starts with "/tmp/x" lexically but is a different
|
||||
// directory. The check must use a trailing separator so "/tmp/x2/..." doesn't
|
||||
// match "/tmp/x".
|
||||
var sibling = Path.GetFileName(_root) + "2";
|
||||
Assert.Throws<UnsafePathException>(
|
||||
() => PathSafety.ResolveInside(_root, $"../{sibling}/evil.txt"));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user