Secure library paths

Emby.Server.Implementations/Library/LibraryManager.cs

@@ -3797,7 +3797,9 @@ namespace Emby.Server.Implementations.Library
             }
 
             var rootFolderPath = _configurationManager.ApplicationPaths.DefaultUserViewsPath;
-            var virtualFolderPath = Path.Combine(rootFolderPath, virtualFolderName);
+            var virtualFolderPath = FileSystemHelper.GetChildPath(rootFolderPath, virtualFolderName)
+                ?? throw new FileNotFoundException(
+                    string.Format(CultureInfo.InvariantCulture, "The media collection {0} does not exist", virtualFolderName));
 
             CreateShortcut(virtualFolderPath, pathInfo);
 
@@ -3818,7 +3820,9 @@ namespace Emby.Server.Implementations.Library
             ArgumentNullException.ThrowIfNull(mediaPath);
 
             var rootFolderPath = _configurationManager.ApplicationPaths.DefaultUserViewsPath;
-            var virtualFolderPath = Path.Combine(rootFolderPath, virtualFolderName);
+            var virtualFolderPath = FileSystemHelper.GetChildPath(rootFolderPath, virtualFolderName)
+                ?? throw new FileNotFoundException(
+                    string.Format(CultureInfo.InvariantCulture, "The media collection {0} does not exist", virtualFolderName));
 
             var libraryOptions = CollectionFolder.GetLibraryOptions(virtualFolderPath);
 
@@ -3857,9 +3861,9 @@ namespace Emby.Server.Implementations.Library
 
             var rootFolderPath = _configurationManager.ApplicationPaths.DefaultUserViewsPath;
 
-            var path = Path.Combine(rootFolderPath, name);
+            var path = FileSystemHelper.GetChildPath(rootFolderPath, name);
 
-            if (!Directory.Exists(path))
+            if (path is null || !Directory.Exists(path))
             {
                 throw new FileNotFoundException("The media folder does not exist");
             }
@@ -3923,9 +3927,9 @@ namespace Emby.Server.Implementations.Library
             ArgumentException.ThrowIfNullOrEmpty(mediaPath);
 
             var rootFolderPath = _configurationManager.ApplicationPaths.DefaultUserViewsPath;
-            var virtualFolderPath = Path.Combine(rootFolderPath, virtualFolderName);
+            var virtualFolderPath = FileSystemHelper.GetChildPath(rootFolderPath, virtualFolderName);
 
-            if (!Directory.Exists(virtualFolderPath))
+            if (virtualFolderPath is null || !Directory.Exists(virtualFolderPath))
             {
                 throw new FileNotFoundException(
                     string.Format(CultureInfo.InvariantCulture, "The media collection {0} does not exist", virtualFolderName));

Jellyfin.Api/Controllers/LibraryStructureController.cs

@@ -14,6 +14,7 @@ using MediaBrowser.Common.Api;
 using MediaBrowser.Controller;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.IO;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Model.Configuration;
 using MediaBrowser.Model.Entities;
@@ -122,12 +123,14 @@ public class LibraryStructureController : BaseJellyfinApiController
     /// <param name="newName">The new name.</param>
     /// <param name="refreshLibrary">Whether to refresh the library.</param>
     /// <response code="204">Folder renamed.</response>
+    /// <response code="400">The new name is not a valid library name.</response>
     /// <response code="404">Library doesn't exist.</response>
     /// <response code="409">Library already exists.</response>
-    /// <returns>A <see cref="NoContentResult"/> on success, a <see cref="NotFoundResult"/> if the library doesn't exist, a <see cref="ConflictResult"/> if the new name is already taken.</returns>
+    /// <returns>A <see cref="NoContentResult"/> on success, a <see cref="BadRequestResult"/> if the new name is invalid, a <see cref="NotFoundResult"/> if the library doesn't exist, a <see cref="ConflictResult"/> if the new name is already taken.</returns>
     /// <exception cref="ArgumentNullException">The new name may not be null.</exception>
     [HttpPost("Name")]
     [ProducesResponseType(StatusCodes.Status204NoContent)]
+    [ProducesResponseType(StatusCodes.Status400BadRequest)]
     [ProducesResponseType(StatusCodes.Status404NotFound)]
     [ProducesResponseType(StatusCodes.Status409Conflict)]
     public ActionResult RenameVirtualFolder(
@@ -147,10 +150,15 @@ public class LibraryStructureController : BaseJellyfinApiController
 
         var rootFolderPath = _appPaths.DefaultUserViewsPath;
 
-        var currentPath = Path.Combine(rootFolderPath, name);
-        var newPath = Path.Combine(rootFolderPath, newName);
+        // Both names are caller supplied, so they have to be confined to the libraries root.
+        var newPath = FileSystemHelper.GetChildPath(rootFolderPath, newName);
+        if (newPath is null)
+        {
+            return BadRequest("The new name is not a valid library name.");
+        }
 
-        if (!Directory.Exists(currentPath))
+        var currentPath = FileSystemHelper.GetChildPath(rootFolderPath, name);
+        if (currentPath is null || !Directory.Exists(currentPath))
         {
             return NotFound("The media collection does not exist.");
         }

MediaBrowser.Controller/IO/FileSystemHelper.cs

@@ -166,4 +166,35 @@ public static class FileSystemHelper
 
         return ResolveLinkTarget(fileInfo.FullName, returnFinalTarget);
     }
+
+    /// <summary>
+    /// Combines a caller supplied name with a parent directory, making sure the name cannot escape that directory.
+    /// </summary>
+    /// <param name="parentPath">The directory the name has to resolve inside of.</param>
+    /// <param name="name">The name of the child.</param>
+    /// <returns>
+    /// The full path of the child, or <c>null</c> if <paramref name="name"/> is not the name of a direct child
+    /// of <paramref name="parentPath"/>.
+    /// </returns>
+    public static string? GetChildPath(string parentPath, string name)
+    {
+        if (string.IsNullOrWhiteSpace(name) || name.Contains('\0', StringComparison.Ordinal))
+        {
+            return null;
+        }
+
+        // Rejects directory separators, and on Windows also volume separators, as those make the name more than a single segment.
+        if (!string.Equals(Path.GetFileName(name), name, StringComparison.Ordinal))
+        {
+            return null;
+        }
+
+        var fullPath = Path.GetFullPath(Path.Combine(parentPath, name));
+        var fullParentPath = Path.TrimEndingDirectorySeparator(Path.GetFullPath(parentPath));
+
+        // Catches the remaining relative names, "." and "..", which are valid single segments.
+        return string.Equals(Path.GetDirectoryName(fullPath), fullParentPath, StringComparison.Ordinal)
+            ? fullPath
+            : null;
+    }
 }

tests/Jellyfin.Controller.Tests/IO/FileSystemHelperTests.cs

@@ -0,0 +1,60 @@
+using System;
+using System.IO;
+using MediaBrowser.Controller.IO;
+using Xunit;
+
+namespace Jellyfin.Controller.Tests.IO;
+
+public class FileSystemHelperTests
+{
+    private static readonly string _parentPath = Path.Combine(Path.GetTempPath(), "jellyfin-test", "root", "default");
+
+    [Theory]
+    [InlineData("Movies")]
+    [InlineData("My Movies")]
+    [InlineData("..2")]
+    [InlineData("...")]
+    [InlineData("a.b")]
+    public void GetChildPath_ValidName_ReturnsPathInsideParent(string name)
+    {
+        var path = FileSystemHelper.GetChildPath(_parentPath, name);
+
+        Assert.Equal(Path.Combine(_parentPath, name), path);
+    }
+
+    [Theory]
+    [InlineData("")]
+    [InlineData("   ")]
+    [InlineData(".")]
+    [InlineData("..")]
+    [InlineData("../..")]
+    [InlineData("../../etc")]
+    [InlineData("Movies/../..")]
+    [InlineData("/var/lib/jellyfin/data")]
+    [InlineData("sub/folder")]
+    [InlineData("with\0null")]
+    public void GetChildPath_EscapingName_ReturnsNull(string name)
+    {
+        Assert.Null(FileSystemHelper.GetChildPath(_parentPath, name));
+    }
+
+    [Theory]
+    [InlineData("..\\..")]
+    [InlineData("sub\\folder")]
+    [InlineData("C:\\Windows")]
+    public void GetChildPath_WindowsSeparator_DoesNotEscapeParent(string name)
+    {
+        var path = FileSystemHelper.GetChildPath(_parentPath, name);
+
+        // On Windows these are rejected outright, on other platforms a backslash is a legal file name character.
+        Assert.True(path is null || string.Equals(Path.GetDirectoryName(path), _parentPath, StringComparison.Ordinal));
+    }
+
+    [Fact]
+    public void GetChildPath_ParentWithTrailingSeparator_ReturnsPathInsideParent()
+    {
+        var path = FileSystemHelper.GetChildPath(_parentPath + Path.DirectorySeparatorChar, "Movies");
+
+        Assert.Equal(Path.Combine(_parentPath, "Movies"), path);
+    }
+}

tests/Jellyfin.Server.Integration.Tests/Controllers/LibraryStructureControllerTests.cs

@@ -114,6 +114,58 @@ public sealed class LibraryStructureControllerTests : IClassFixture<JellyfinAppl
         Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
     }
 
+    [Theory]
+    [Priority(1)]
+    [InlineData("..")]
+    [InlineData("../..")]
+    [InlineData(".")]
+    [InlineData("test/../..")]
+    [InlineData("/var/lib/jellyfin/data")]
+    public async Task DeleteLibrary_PathTraversal_NotFound(string name)
+    {
+        var client = _factory.CreateClient();
+        client.DefaultRequestHeaders.AddAuthHeader(_accessToken ??= await AuthHelper.CompleteStartupAsync(client));
+
+        using var response = await client.DeleteAsync($"Library/VirtualFolders?name={Uri.EscapeDataString(name)}", TestContext.Current.CancellationToken);
+        Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
+    }
+
+    [Theory]
+    [Priority(1)]
+    [InlineData("..")]
+    [InlineData("../..")]
+    [InlineData(".")]
+    [InlineData("test/../..")]
+    [InlineData("/var/lib/jellyfin/data")]
+    public async Task RenameLibrary_PathTraversalNewName_BadRequest(string newName)
+    {
+        var client = _factory.CreateClient();
+        client.DefaultRequestHeaders.AddAuthHeader(_accessToken ??= await AuthHelper.CompleteStartupAsync(client));
+
+        using var response = await client.PostAsync(
+            $"Library/VirtualFolders/Name?name=test&newName={Uri.EscapeDataString(newName)}",
+            null,
+            TestContext.Current.CancellationToken);
+        Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
+    }
+
+    [Theory]
+    [Priority(1)]
+    [InlineData("..")]
+    [InlineData("../..")]
+    [InlineData("/var/lib/jellyfin/data")]
+    public async Task RenameLibrary_PathTraversalName_NotFound(string name)
+    {
+        var client = _factory.CreateClient();
+        client.DefaultRequestHeaders.AddAuthHeader(_accessToken ??= await AuthHelper.CompleteStartupAsync(client));
+
+        using var response = await client.PostAsync(
+            $"Library/VirtualFolders/Name?name={Uri.EscapeDataString(name)}&newName=renamed",
+            null,
+            TestContext.Current.CancellationToken);
+        Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
+    }
+
     [Fact]
     [Priority(1)]
     public async Task DeleteLibrary_Valid_Success()