From 617ebf367ff24ecbe8e0de5aebc90fe28689bcb0 Mon Sep 17 00:00:00 2001 From: Marc Brooks Date: Wed, 1 Apr 2026 16:55:47 -0500 Subject: Fix path transversal exposure in Plugins The request path is not validated to a valid path and could allow escaping the transcode path and downloading of any arbitrary file in GetHlsPlaylistLegacy . GetHlsAudioSegmentLegacy and GetHlsVideoSegmentLegacy have the same issue, and are NOT behind an Authorize so they are publicly exploitable. Added a ValidateTranscodePath that verifies that requested file paths start with the transcode path setting. Also ensure that all filename comparisons are OrdinalIgnoreCase because we might be running on a filesystem where filename-casing doesn't have to match. Switched from InvariantCulture because the underlying OS filename comparisons are always byte-wise (with case insensitivity here). Fixed a similar issue in GetPluginImage --- Jellyfin.Api/Controllers/HlsSegmentController.cs | 42 +++++++++++++----------- Jellyfin.Api/Controllers/PluginsController.cs | 7 ++-- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/Jellyfin.Api/Controllers/HlsSegmentController.cs b/Jellyfin.Api/Controllers/HlsSegmentController.cs index b5365cd632..4cb7627559 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 = _serverConfigurationManager.GetTranscodePath(); + var file = Path.GetFullPath(filename, transcodePath); + var fileDir = Path.GetDirectoryName(file); + if (string.IsNullOrEmpty(fileDir) || !fileDir.StartsWith(transcodePath, 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..5f6136ed11 100644 --- a/Jellyfin.Api/Controllers/PluginsController.cs +++ b/Jellyfin.Api/Controllers/PluginsController.cs @@ -226,10 +226,11 @@ 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)) + imagePath = Path.GetFullPath(imagePath, plugin.Path); + if (imagePath.StartsWith(plugin.Path, StringComparison.OrdinalIgnoreCase) is false || System.IO.File.Exists(imagePath) is false) { return NotFound(); } -- cgit v1.2.3 From 95cebffa8782e8ce8dc48a15c9723ce9f33cb09c Mon Sep 17 00:00:00 2001 From: Marc Brooks Date: Sun, 28 Jun 2026 16:26:35 -0500 Subject: Add tests Also fixed a sibling directory that matches the prefix. --- Jellyfin.Api/Controllers/HlsSegmentController.cs | 6 +- Jellyfin.Api/Controllers/PluginsController.cs | 6 +- .../Controllers/HlsSegmentControllerTests.cs | 161 +++++++++++++++++++++ .../Controllers/PluginsControllerTests.cs | 129 +++++++++++++++++ 4 files changed, 297 insertions(+), 5 deletions(-) create mode 100644 tests/Jellyfin.Api.Tests/Controllers/HlsSegmentControllerTests.cs create mode 100644 tests/Jellyfin.Api.Tests/Controllers/PluginsControllerTests.cs diff --git a/Jellyfin.Api/Controllers/HlsSegmentController.cs b/Jellyfin.Api/Controllers/HlsSegmentController.cs index 4cb7627559..c61ee45830 100644 --- a/Jellyfin.Api/Controllers/HlsSegmentController.cs +++ b/Jellyfin.Api/Controllers/HlsSegmentController.cs @@ -164,10 +164,10 @@ public class HlsSegmentController : BaseJellyfinApiController private string? ValidateTranscodePath(string filename) { - var transcodePath = _serverConfigurationManager.GetTranscodePath(); + var transcodePath = Path.TrimEndingDirectorySeparator(Path.GetFullPath(_serverConfigurationManager.GetTranscodePath())); var file = Path.GetFullPath(filename, transcodePath); - var fileDir = Path.GetDirectoryName(file); - if (string.IsNullOrEmpty(fileDir) || !fileDir.StartsWith(transcodePath, StringComparison.OrdinalIgnoreCase)) + // Require a separator after the transcode path so a sibling like "-evil" can't pass. + if (!file.StartsWith(transcodePath + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase)) { return null; } diff --git a/Jellyfin.Api/Controllers/PluginsController.cs b/Jellyfin.Api/Controllers/PluginsController.cs index 5f6136ed11..2c84fde972 100644 --- a/Jellyfin.Api/Controllers/PluginsController.cs +++ b/Jellyfin.Api/Controllers/PluginsController.cs @@ -229,8 +229,10 @@ public class PluginsController : BaseJellyfinApiController string? imagePath = plugin.Manifest.ImagePath; if (!string.IsNullOrWhiteSpace(imagePath)) { - imagePath = Path.GetFullPath(imagePath, plugin.Path); - if (imagePath.StartsWith(plugin.Path, StringComparison.OrdinalIgnoreCase) is false || System.IO.File.Exists(imagePath) is false) + var pluginPath = Path.TrimEndingDirectorySeparator(Path.GetFullPath(plugin.Path)); + imagePath = Path.GetFullPath(imagePath, pluginPath); + // Require a separator after the plugin path so a sibling like "-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 _fileSystem = new(); + private readonly Mock _config = new(); + private readonly Mock _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()); + } + + 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(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(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(result); + } + + [Fact] + public void GetHlsAudioSegmentLegacy_SiblingPrefixDirectory_ReturnsBadRequest() + { + var controller = CreateController("/Audio/abc/hls/segment/stream.mp3"); + + // Resolves to "-evil/passwd", which shares the transcode path as a string prefix. + var result = controller.GetHlsAudioSegmentLegacy("abc", "../jellyfin-hls-segment-tests-evil/passwd"); + + Assert.IsType(result); + } + + [Fact] + public void GetHlsPlaylistLegacy_M3u8InsideTranscodePath_ReturnsFile() + { + var controller = CreateController("/Videos/abc/hls/list/stream.m3u8"); + + var result = controller.GetHlsPlaylistLegacy("abc", "list"); + + Assert.IsType(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(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(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(result); + } + + [Fact] + public void GetHlsVideoSegmentLegacy_NoMatchingPlaylist_ReturnsNotFound() + { + _fileSystem.Setup(f => f.GetFilePaths(_transcodePath, false)) + .Returns(Array.Empty()); + + var controller = CreateController("/Videos/abc/hls/playlist123/seg1.ts"); + + var result = controller.GetHlsVideoSegmentLegacy("abc", "playlist123", "seg1", "ts"); + + Assert.IsType(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(result); + _fileSystem.Verify(f => f.GetFilePaths(It.IsAny(), It.IsAny()), 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 _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(), _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(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()); + SetupPlugin(id, version, "logo.png"); + + var result = CreateController().GetPluginImage(id, version); + + Assert.IsType(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(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(result); + } + + [Fact] + public void GetPluginImage_SiblingPrefixDirectory_ReturnsNotFound() + { + var id = Guid.NewGuid(); + var version = new Version(1, 0); + // Resolves to "-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()); + SetupPlugin(id, version, "../jellyfin-plugin-image-tests-evil/logo.png"); + + var result = CreateController().GetPluginImage(id, version); + + Assert.IsType(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(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(result); + } +} -- cgit v1.2.3