Merge pull request #17191 from IDisposable/fix/handler-path-traversal

Fix path transversal exposure in Plugins

4 files changed, 318 insertions, 23 deletions

diff --git a/Jellyfin.Api/Controllers/HlsSegmentController.cs b/Jellyfin.Api/Controllers/HlsSegmentController.cs
index b5365cd632..c61ee45830 100644
--- a/Jellyfin.Api/Controllers/HlsSegmentController.cs
+++ b/Jellyfin.Api/Controllers/HlsSegmentController.cs
@@ -60,11 +60,8 @@ public class HlsSegmentController : BaseJellyfinApiController
     public ActionResult GetHlsAudioSegmentLegacy([FromRoute, Required] string itemId, [FromRoute, Required] string segmentId)
     {
         // TODO: Deprecate with new iOS app
-        var file = string.Concat(segmentId, Path.GetExtension(Request.Path.Value.AsSpan()));
-        var transcodePath = _serverConfigurationManager.GetTranscodePath();
-        file = Path.GetFullPath(Path.Combine(transcodePath, file));
-        var fileDir = Path.GetDirectoryName(file);
-        if (string.IsNullOrEmpty(fileDir) || !fileDir.StartsWith(transcodePath, StringComparison.InvariantCulture))
+        var file = ValidateTranscodePath(string.Concat(segmentId, Path.GetExtension(Request.Path.Value.AsSpan())));
+        if (file is null)
         {
             return BadRequest("Invalid segment.");
         }
@@ -86,12 +83,9 @@ public class HlsSegmentController : BaseJellyfinApiController
     [SuppressMessage("Microsoft.Performance", "CA1801:ReviewUnusedParameters", MessageId = "itemId", Justification = "Required for ServiceStack")]
     public ActionResult GetHlsPlaylistLegacy([FromRoute, Required] string itemId, [FromRoute, Required] string playlistId)
     {
-        var file = string.Concat(playlistId, Path.GetExtension(Request.Path.Value.AsSpan()));
-        var transcodePath = _serverConfigurationManager.GetTranscodePath();
-        file = Path.GetFullPath(Path.Combine(transcodePath, file));
-        var fileDir = Path.GetDirectoryName(file);
-        if (string.IsNullOrEmpty(fileDir) || !fileDir.StartsWith(transcodePath, StringComparison.InvariantCulture)
-            || Path.GetExtension(file.AsSpan()).Equals(".m3u8", StringComparison.OrdinalIgnoreCase))
+        var file = ValidateTranscodePath(string.Concat(playlistId, Path.GetExtension(Request.Path.Value.AsSpan())));
+        if (file is null
+            || !Path.GetExtension(file.AsSpan()).Equals(".m3u8", StringComparison.OrdinalIgnoreCase))
         {
             return BadRequest("Invalid segment.");
         }
@@ -140,18 +134,13 @@ public class HlsSegmentController : BaseJellyfinApiController
         [FromRoute, Required] string segmentId,
         [FromRoute, Required] string segmentContainer)
     {
-        var file = string.Concat(segmentId, Path.GetExtension(Request.Path.Value.AsSpan()));
-        var transcodeFolderPath = _serverConfigurationManager.GetTranscodePath();
-
-        file = Path.GetFullPath(Path.Combine(transcodeFolderPath, file));
-        var fileDir = Path.GetDirectoryName(file);
-        if (string.IsNullOrEmpty(fileDir) || !fileDir.StartsWith(transcodeFolderPath, StringComparison.InvariantCulture))
+        var file = ValidateTranscodePath(string.Concat(segmentId, Path.GetExtension(Request.Path.Value.AsSpan())));
+        if (file is null)
         {
             return BadRequest("Invalid segment.");
         }
 
-        var normalizedPlaylistId = playlistId;
-
+        var transcodeFolderPath = _serverConfigurationManager.GetTranscodePath();
         var filePaths = _fileSystem.GetFilePaths(transcodeFolderPath);
         // Add . to start of segment container for future use.
         segmentContainer = segmentContainer.Insert(0, ".");
@@ -161,7 +150,7 @@ public class HlsSegmentController : BaseJellyfinApiController
             var pathExtension = Path.GetExtension(path);
             if ((string.Equals(pathExtension, segmentContainer, StringComparison.OrdinalIgnoreCase)
                  || string.Equals(pathExtension, ".m3u8", StringComparison.OrdinalIgnoreCase))
-                && path.Contains(normalizedPlaylistId, StringComparison.OrdinalIgnoreCase))
+                && path.Contains(playlistId, StringComparison.OrdinalIgnoreCase))
             {
                 playlistPath = path;
                 break;
@@ -173,6 +162,19 @@ public class HlsSegmentController : BaseJellyfinApiController
             : GetFileResult(file, playlistPath);
     }
 
+    private string? ValidateTranscodePath(string filename)
+    {
+        var transcodePath = Path.TrimEndingDirectorySeparator(Path.GetFullPath(_serverConfigurationManager.GetTranscodePath()));
+        var file = Path.GetFullPath(filename, transcodePath);
+        // Require a separator after the transcode path so a sibling like "<transcodePath>-evil" can't pass.
+        if (!file.StartsWith(transcodePath + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase))
+        {
+            return null;
+        }
+
+        return file;
+    }
+
     private ActionResult GetFileResult(string path, string playlistPath)
     {
         var transcodingJob = _transcodeManager.OnTranscodeBeginRequest(playlistPath, TranscodingJobType.Hls);
diff --git a/Jellyfin.Api/Controllers/PluginsController.cs b/Jellyfin.Api/Controllers/PluginsController.cs
index 0105ecf7a7..2c84fde972 100644
--- a/Jellyfin.Api/Controllers/PluginsController.cs
+++ b/Jellyfin.Api/Controllers/PluginsController.cs
@@ -226,10 +226,13 @@ public class PluginsController : BaseJellyfinApiController
             return NotFound();
         }
 
-        if (!string.IsNullOrEmpty(plugin.Manifest.ImagePath))
+        string? imagePath = plugin.Manifest.ImagePath;
+        if (!string.IsNullOrWhiteSpace(imagePath))
         {
-            var imagePath = Path.Combine(plugin.Path, plugin.Manifest.ImagePath);
-            if (!System.IO.File.Exists(imagePath))
+            var pluginPath = Path.TrimEndingDirectorySeparator(Path.GetFullPath(plugin.Path));
+            imagePath = Path.GetFullPath(imagePath, pluginPath);
+            // Require a separator after the plugin path so a sibling like "<pluginPath>-evil" can't pass.
+            if (imagePath.StartsWith(pluginPath + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase) is false || System.IO.File.Exists(imagePath) is false)
             {
                 return NotFound();
             }
diff --git a/tests/Jellyfin.Api.Tests/Controllers/HlsSegmentControllerTests.cs b/tests/Jellyfin.Api.Tests/Controllers/HlsSegmentControllerTests.cs
new file mode 100644
index 0000000000..a248664928
--- /dev/null
+++ b/tests/Jellyfin.Api.Tests/Controllers/HlsSegmentControllerTests.cs
@@ -0,0 +1,161 @@
+using System;
+using System.IO;
+using Jellyfin.Api.Controllers;
+using MediaBrowser.Common.Configuration;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.MediaEncoding;
+using MediaBrowser.Model.Configuration;
+using MediaBrowser.Model.IO;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Api.Tests.Controllers;
+
+// The legacy HLS endpoints build a file path from caller-supplied route values, and the audio
+// and video segment endpoints are not authenticated. These tests pin down that requests escaping
+// the transcode directory are rejected while legitimate ones still serve a file.
+public sealed class HlsSegmentControllerTests
+{
+    private readonly Mock<IFileSystem> _fileSystem = new();
+    private readonly Mock<IServerConfigurationManager> _config = new();
+    private readonly Mock<ITranscodeManager> _transcodeManager = new();
+    private readonly string _transcodePath;
+
+    public HlsSegmentControllerTests()
+    {
+        _transcodePath = Path.Combine(Path.GetTempPath(), "jellyfin-hls-segment-tests");
+        Directory.CreateDirectory(_transcodePath);
+
+        _config.Setup(c => c.GetConfiguration("encoding"))
+            .Returns(new EncodingOptions { TranscodingTempPath = _transcodePath });
+        _config.SetupGet(c => c.CommonApplicationPaths).Returns(Mock.Of<IApplicationPaths>());
+    }
+
+    private HlsSegmentController CreateController(string requestPath)
+    {
+        var httpContext = new DefaultHttpContext();
+        httpContext.Request.Path = requestPath;
+
+        return new HlsSegmentController(_fileSystem.Object, _config.Object, _transcodeManager.Object)
+        {
+            ControllerContext = new ControllerContext { HttpContext = httpContext }
+        };
+    }
+
+    [Fact]
+    public void GetHlsAudioSegmentLegacy_SegmentInsideTranscodePath_ReturnsFile()
+    {
+        var controller = CreateController("/Audio/abc/hls/segment/stream.mp3");
+
+        var result = controller.GetHlsAudioSegmentLegacy("abc", "segment");
+
+        Assert.IsType<PhysicalFileResult>(result);
+    }
+
+    [Theory]
+    [InlineData("../../../../etc/passwd")]
+    [InlineData("subdir/../../../../etc/passwd")]
+    public void GetHlsAudioSegmentLegacy_TraversalOutsideTranscodePath_ReturnsBadRequest(string segmentId)
+    {
+        var controller = CreateController("/Audio/abc/hls/segment/stream.mp3");
+
+        var result = controller.GetHlsAudioSegmentLegacy("abc", segmentId);
+
+        Assert.IsType<BadRequestObjectResult>(result);
+    }
+
+    [Fact]
+    public void GetHlsAudioSegmentLegacy_AbsoluteRootedPath_ReturnsBadRequest()
+    {
+        var controller = CreateController("/Audio/abc/hls/segment/stream.mp3");
+
+        // A rooted segment id makes Path.GetFullPath discard the transcode base.
+        var rooted = OperatingSystem.IsWindows() ? "C:\\Windows\\win.ini" : "/etc/passwd";
+        var result = controller.GetHlsAudioSegmentLegacy("abc", rooted);
+
+        Assert.IsType<BadRequestObjectResult>(result);
+    }
+
+    [Fact]
+    public void GetHlsAudioSegmentLegacy_SiblingPrefixDirectory_ReturnsBadRequest()
+    {
+        var controller = CreateController("/Audio/abc/hls/segment/stream.mp3");
+
+        // Resolves to "<transcodePath>-evil/passwd", which shares the transcode path as a string prefix.
+        var result = controller.GetHlsAudioSegmentLegacy("abc", "../jellyfin-hls-segment-tests-evil/passwd");
+
+        Assert.IsType<BadRequestObjectResult>(result);
+    }
+
+    [Fact]
+    public void GetHlsPlaylistLegacy_M3u8InsideTranscodePath_ReturnsFile()
+    {
+        var controller = CreateController("/Videos/abc/hls/list/stream.m3u8");
+
+        var result = controller.GetHlsPlaylistLegacy("abc", "list");
+
+        Assert.IsType<PhysicalFileResult>(result);
+    }
+
+    [Fact]
+    public void GetHlsPlaylistLegacy_NonPlaylistExtension_ReturnsBadRequest()
+    {
+        // Playlist endpoint serves only .m3u8, even for a path inside the transcode dir.
+        var controller = CreateController("/Videos/abc/hls/list/stream.mp4");
+
+        var result = controller.GetHlsPlaylistLegacy("abc", "list");
+
+        Assert.IsType<BadRequestObjectResult>(result);
+    }
+
+    [Theory]
+    [InlineData("../../../../etc/passwd")]
+    public void GetHlsPlaylistLegacy_TraversalOutsideTranscodePath_ReturnsBadRequest(string playlistId)
+    {
+        var controller = CreateController("/Videos/abc/hls/list/stream.m3u8");
+
+        var result = controller.GetHlsPlaylistLegacy("abc", playlistId);
+
+        Assert.IsType<BadRequestObjectResult>(result);
+    }
+
+    [Fact]
+    public void GetHlsVideoSegmentLegacy_SegmentInsideTranscodePath_ReturnsFile()
+    {
+        _fileSystem.Setup(f => f.GetFilePaths(_transcodePath, false))
+            .Returns(new[] { Path.Combine(_transcodePath, "playlist123.ts") });
+
+        var controller = CreateController("/Videos/abc/hls/playlist123/seg1.ts");
+
+        var result = controller.GetHlsVideoSegmentLegacy("abc", "playlist123", "seg1", "ts");
+
+        Assert.IsType<PhysicalFileResult>(result);
+    }
+
+    [Fact]
+    public void GetHlsVideoSegmentLegacy_NoMatchingPlaylist_ReturnsNotFound()
+    {
+        _fileSystem.Setup(f => f.GetFilePaths(_transcodePath, false))
+            .Returns(Array.Empty<string>());
+
+        var controller = CreateController("/Videos/abc/hls/playlist123/seg1.ts");
+
+        var result = controller.GetHlsVideoSegmentLegacy("abc", "playlist123", "seg1", "ts");
+
+        Assert.IsType<NotFoundObjectResult>(result);
+    }
+
+    [Theory]
+    [InlineData("../../../../etc/passwd")]
+    public void GetHlsVideoSegmentLegacy_TraversalOutsideTranscodePath_ReturnsBadRequest(string segmentId)
+    {
+        var controller = CreateController("/Videos/abc/hls/playlist123/seg1.ts");
+
+        var result = controller.GetHlsVideoSegmentLegacy("abc", "playlist123", segmentId, "ts");
+
+        Assert.IsType<BadRequestObjectResult>(result);
+        _fileSystem.Verify(f => f.GetFilePaths(It.IsAny<string>(), It.IsAny<bool>()), Times.Never);
+    }
+}
diff --git a/tests/Jellyfin.Api.Tests/Controllers/PluginsControllerTests.cs b/tests/Jellyfin.Api.Tests/Controllers/PluginsControllerTests.cs
new file mode 100644
index 0000000000..f040a328bb
--- /dev/null
+++ b/tests/Jellyfin.Api.Tests/Controllers/PluginsControllerTests.cs
@@ -0,0 +1,129 @@
+using System;
+using System.IO;
+using Jellyfin.Api.Controllers;
+using MediaBrowser.Common.Plugins;
+using MediaBrowser.Common.Updates;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Api.Tests.Controllers;
+
+// Covers the path-traversal validation in GetPluginImage: a plugin's manifest ImagePath
+// must resolve to a file inside the plugin's own directory.
+public sealed class PluginsControllerTests
+{
+    private readonly Mock<IPluginManager> _pluginManager = new();
+    private readonly string _pluginPath;
+
+    public PluginsControllerTests()
+    {
+        _pluginPath = Path.Combine(Path.GetTempPath(), "jellyfin-plugin-image-tests");
+        Directory.CreateDirectory(_pluginPath);
+    }
+
+    private PluginsController CreateController() =>
+        new PluginsController(Mock.Of<IInstallationManager>(), _pluginManager.Object)
+        {
+            ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() }
+        };
+
+    private void SetupPlugin(Guid id, Version version, string? imagePath)
+    {
+        var manifest = new PluginManifest { Id = id, Name = "Test", Version = version.ToString(), ImagePath = imagePath };
+        _pluginManager.Setup(p => p.GetPlugin(id, version))
+            .Returns(new LocalPlugin(_pluginPath, true, manifest));
+    }
+
+    [Fact]
+    public void GetPluginImage_UnknownPlugin_ReturnsNotFound()
+    {
+        var result = CreateController().GetPluginImage(Guid.NewGuid(), new Version(1, 0));
+
+        Assert.IsType<NotFoundResult>(result);
+    }
+
+    [Fact]
+    public void GetPluginImage_ImageInsidePluginPath_ReturnsFile()
+    {
+        var id = Guid.NewGuid();
+        var version = new Version(1, 0);
+        File.WriteAllBytes(Path.Combine(_pluginPath, "logo.png"), Array.Empty<byte>());
+        SetupPlugin(id, version, "logo.png");
+
+        var result = CreateController().GetPluginImage(id, version);
+
+        Assert.IsType<PhysicalFileResult>(result);
+    }
+
+    [Fact]
+    public void GetPluginImage_ImageInsidePluginPathButMissing_ReturnsNotFound()
+    {
+        var id = Guid.NewGuid();
+        var version = new Version(1, 0);
+        SetupPlugin(id, version, "does-not-exist.png");
+
+        var result = CreateController().GetPluginImage(id, version);
+
+        Assert.IsType<NotFoundResult>(result);
+    }
+
+    [Theory]
+    [InlineData("../../../../etc/passwd")]
+    [InlineData("subdir/../../../../etc/passwd")]
+    public void GetPluginImage_TraversalOutsidePluginPath_ReturnsNotFound(string imagePath)
+    {
+        var id = Guid.NewGuid();
+        var version = new Version(1, 0);
+        SetupPlugin(id, version, imagePath);
+
+        var result = CreateController().GetPluginImage(id, version);
+
+        Assert.IsType<NotFoundResult>(result);
+    }
+
+    [Fact]
+    public void GetPluginImage_SiblingPrefixDirectory_ReturnsNotFound()
+    {
+        var id = Guid.NewGuid();
+        var version = new Version(1, 0);
+        // Resolves to "<pluginPath>-evil/logo.png", which shares the plugin path as a string prefix.
+        // The file is created so the check fails on the boundary, not on File.Exists.
+        var siblingDir = _pluginPath + "-evil";
+        Directory.CreateDirectory(siblingDir);
+        File.WriteAllBytes(Path.Combine(siblingDir, "logo.png"), Array.Empty<byte>());
+        SetupPlugin(id, version, "../jellyfin-plugin-image-tests-evil/logo.png");
+
+        var result = CreateController().GetPluginImage(id, version);
+
+        Assert.IsType<NotFoundResult>(result);
+    }
+
+    [Fact]
+    public void GetPluginImage_AbsoluteImagePath_ReturnsNotFound()
+    {
+        var id = Guid.NewGuid();
+        var version = new Version(1, 0);
+        SetupPlugin(id, version, OperatingSystem.IsWindows() ? "C:\\Windows\\win.ini" : "/etc/passwd");
+
+        var result = CreateController().GetPluginImage(id, version);
+
+        Assert.IsType<NotFoundResult>(result);
+    }
+
+    [Theory]
+    [InlineData(null)]
+    [InlineData("")]
+    [InlineData("   ")]
+    public void GetPluginImage_NoImagePathOrResource_ReturnsNotFound(string? imagePath)
+    {
+        var id = Guid.NewGuid();
+        var version = new Version(1, 0);
+        SetupPlugin(id, version, imagePath);
+
+        var result = CreateController().GetPluginImage(id, version);
+
+        Assert.IsType<NotFoundResult>(result);
+    }
+}