Merge pull request #17368 from Shadowghost/security-path-traversal-fixes

Backport and extend path traversal fixes

3 files changed, 106 insertions, 0 deletions

diff --git a/tests/Jellyfin.Controller.Tests/ClientEventLoggerTests.cs b/tests/Jellyfin.Controller.Tests/ClientEventLoggerTests.cs
new file mode 100644
index 0000000000..5132e529dd
--- /dev/null
+++ b/tests/Jellyfin.Controller.Tests/ClientEventLoggerTests.cs
@@ -0,0 +1,44 @@
+using System;
+using System.IO;
+using System.Text;
+using System.Threading.Tasks;
+using MediaBrowser.Controller;
+using MediaBrowser.Controller.ClientEvent;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Controller.Tests
+{
+    public class ClientEventLoggerTests
+    {
+        [Theory]
+        [InlineData("../../../../etc/passwd", "1.0")]
+        [InlineData("..\\..\\windows\\system32", "1.0")]
+        [InlineData("normal-client", "../../../etc/passwd")]
+        [InlineData("/absolute/path", "1.0")]
+        public async Task WriteDocumentAsync_TraversalInput_StaysInsideLogDirectory(string clientName, string clientVersion)
+        {
+            var logDir = Path.Combine(Path.GetTempPath(), "jellyfin-clientlog-test-" + Path.GetRandomFileName());
+            Directory.CreateDirectory(logDir);
+            try
+            {
+                var paths = new Mock<IServerApplicationPaths>();
+                paths.Setup(p => p.LogDirectoryPath).Returns(logDir);
+
+                var logger = new ClientEventLogger(paths.Object);
+                using var contents = new MemoryStream(Encoding.UTF8.GetBytes("payload"));
+
+                var fileName = await logger.WriteDocumentAsync(clientName, clientVersion, contents);
+
+                var resolved = Path.GetFullPath(Path.Combine(logDir, fileName));
+                var rootWithSep = Path.GetFullPath(logDir) + Path.DirectorySeparatorChar;
+                Assert.StartsWith(rootWithSep, resolved, StringComparison.Ordinal);
+                Assert.True(File.Exists(resolved));
+            }
+            finally
+            {
+                Directory.Delete(logDir, recursive: true);
+            }
+        }
+    }
+}
diff --git a/tests/Jellyfin.Extensions.Tests/PathHelperTests.cs b/tests/Jellyfin.Extensions.Tests/PathHelperTests.cs
new file mode 100644
index 0000000000..71fd853ba2
--- /dev/null
+++ b/tests/Jellyfin.Extensions.Tests/PathHelperTests.cs
@@ -0,0 +1,60 @@
+using System.IO;
+using Jellyfin.Extensions;
+using Xunit;
+
+namespace Jellyfin.Extensions.Tests
+{
+    public static class PathHelperTests
+    {
+        [Theory]
+        [InlineData("file.txt", "file.txt")]
+        [InlineData("sub/file.txt", "file.txt")]
+        [InlineData("../../etc/passwd", "passwd")]
+        public static void GetSafeLeafFileName_ReducesToLeaf(string input, string expected)
+        {
+            Assert.Equal(expected, PathHelper.GetSafeLeafFileName(input));
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData("")]
+        [InlineData(".")]
+        [InlineData("..")]
+        public static void GetSafeLeafFileName_RejectsUnusableLeaf(string? input)
+        {
+            Assert.Null(PathHelper.GetSafeLeafFileName(input));
+        }
+
+        [Fact]
+        public static void IsContainedIn_ChildPath_ReturnsTrue()
+        {
+            var root = Path.Combine(Path.GetTempPath(), "root");
+            var child = Path.Combine(root, "sub", "file.txt");
+            Assert.True(PathHelper.IsContainedIn(root, child));
+        }
+
+        [Fact]
+        public static void IsContainedIn_RootItself_ReturnsTrue()
+        {
+            var root = Path.Combine(Path.GetTempPath(), "root");
+            Assert.True(PathHelper.IsContainedIn(root, root));
+        }
+
+        [Fact]
+        public static void IsContainedIn_TraversalEscape_ReturnsFalse()
+        {
+            var root = Path.Combine(Path.GetTempPath(), "root");
+            var escape = Path.Combine(root, "..", "..", "etc", "passwd");
+            Assert.False(PathHelper.IsContainedIn(root, escape));
+        }
+
+        [Fact]
+        public static void IsContainedIn_SiblingPrefixCollision_ReturnsFalse()
+        {
+            // "/var/data" must not be accepted as a parent of "/var/dataset".
+            var root = Path.Combine(Path.GetTempPath(), "data");
+            var sibling = Path.Combine(Path.GetTempPath(), "dataset", "file.txt");
+            Assert.False(PathHelper.IsContainedIn(root, sibling));
+        }
+    }
+}
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerTests.cs
index 4cea53bd3d..2bf1d1d05b 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerTests.cs
@@ -27,6 +27,8 @@ namespace Jellyfin.Server.Implementations.Tests.Users
         [InlineData(" thishasaspaceatthestart")]
         [InlineData(" thishasaspaceatbothends ")]
         [InlineData(" this has a space at both ends and inbetween ")]
+        [InlineData(".")]
+        [InlineData("..")]
         public void ThrowIfInvalidUsername_WhenInvalidUsername_ThrowsArgumentException(string username)
         {
             Assert.Throws<ArgumentException>(() => UserManager.ThrowIfInvalidUsername(username));