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/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);
+ }
+}
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);
+ }
+ }
+ }
+}