From 89d32a9525b71544497eb5d1788edd0c2c53a860 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Fri, 22 May 2026 22:14:38 +0200 Subject: [PATCH 1/2] Add PathHelper --- src/Jellyfin.Extensions/PathHelper.cs | 77 +++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 src/Jellyfin.Extensions/PathHelper.cs diff --git a/src/Jellyfin.Extensions/PathHelper.cs b/src/Jellyfin.Extensions/PathHelper.cs new file mode 100644 index 0000000000..f519cbb651 --- /dev/null +++ b/src/Jellyfin.Extensions/PathHelper.cs @@ -0,0 +1,77 @@ +using System; +using System.IO; + +namespace Jellyfin.Extensions; + +/// +/// Helpers for safely composing filesystem paths from untrusted input. +/// +/// +/// has two issues that matter in +/// any code that joins a trusted directory with an externally-supplied name: +/// it neither normalises .. nor rejects a rooted second argument +/// (a rooted second arg silently discards the first). Use the helpers below +/// any time the name comes from media metadata, request input, archive +/// entries, or any other channel that can be influenced by a third party. +/// +public static class PathHelper +{ + /// + /// Reduces a possibly-untrusted file name to a safe leaf-only name with no + /// directory components. + /// + /// The candidate file name. + /// + /// The leaf component of , or null if + /// the input has no usable leaf (empty, ., or ..). + /// + public static string? GetSafeLeafFileName(string? fileName) + { + if (string.IsNullOrEmpty(fileName)) + { + return null; + } + + var leaf = Path.GetFileName(fileName); + if (string.IsNullOrEmpty(leaf) || leaf == "." || leaf == "..") + { + return null; + } + + return leaf; + } + + /// + /// Returns whether resolves to a path that + /// equals or is contained inside . + /// + /// The directory the candidate must remain inside. + /// The candidate absolute or relative path. + /// true if the candidate is inside or equal to root; otherwise false. + /// + /// Both arguments are resolved via + /// so .. segments are collapsed before the comparison. The root is + /// compared with a trailing directory separator to prevent prefix + /// collisions (e.g. /var/data must not be accepted as a parent of + /// /var/dataset). + /// + public static bool IsContainedIn(string root, string candidate) + { + ArgumentException.ThrowIfNullOrEmpty(root); + ArgumentException.ThrowIfNullOrEmpty(candidate); + + var fullRoot = Path.GetFullPath(root); + var fullCandidate = Path.GetFullPath(candidate); + + if (string.Equals(fullCandidate, fullRoot, StringComparison.Ordinal)) + { + return true; + } + + var rootWithSep = fullRoot.EndsWith(Path.DirectorySeparatorChar) + ? fullRoot + : fullRoot + Path.DirectorySeparatorChar; + + return fullCandidate.StartsWith(rootWithSep, StringComparison.Ordinal); + } +} From 734145ab98ab0d0214c4a5a1072c18e3e67398f4 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Fri, 22 May 2026 22:17:26 +0200 Subject: [PATCH 2/2] Fix GHSA-jg92-mrxq-vv75 --- .../ClientEvent/ClientEventLogger.cs | 10 ++++- .../ClientEventLoggerTests.cs | 44 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/Jellyfin.Controller.Tests/ClientEventLoggerTests.cs diff --git a/MediaBrowser.Controller/ClientEvent/ClientEventLogger.cs b/MediaBrowser.Controller/ClientEvent/ClientEventLogger.cs index 14dc64dabd..36f0d2195c 100644 --- a/MediaBrowser.Controller/ClientEvent/ClientEventLogger.cs +++ b/MediaBrowser.Controller/ClientEvent/ClientEventLogger.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Threading.Tasks; +using Jellyfin.Extensions; namespace MediaBrowser.Controller.ClientEvent { @@ -21,8 +22,15 @@ namespace MediaBrowser.Controller.ClientEvent /// public async Task WriteDocumentAsync(string clientName, string clientVersion, Stream fileContents) { - var fileName = $"upload_{clientName}_{clientVersion}_{DateTime.UtcNow:yyyyMMddHHmmss}_{Guid.NewGuid():N}.log"; + var safeClientName = PathHelper.GetSafeLeafFileName(clientName) ?? "unknown-client"; + var safeClientVersion = PathHelper.GetSafeLeafFileName(clientVersion) ?? "unknown-version"; + var fileName = $"upload_{safeClientName}_{safeClientVersion}_{DateTime.UtcNow:yyyyMMddHHmmss}_{Guid.NewGuid():N}.log"; var logFilePath = Path.Combine(_applicationPaths.LogDirectoryPath, fileName); + if (!PathHelper.IsContainedIn(_applicationPaths.LogDirectoryPath, logFilePath)) + { + throw new ArgumentException("Path resolved to filename not in log directory"); + } + var fileStream = new FileStream(logFilePath, FileMode.CreateNew, FileAccess.Write, FileShare.None); await using (fileStream.ConfigureAwait(false)) { 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(); + 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); + } + } + } +}