Merge commit from fork

Fix GHSA-f47c-m7gr-q92j

Emby.Server.Implementations/Library/PathManager.cs

@@ -2,10 +2,12 @@ using System;
 using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
+using Jellyfin.Extensions;
 using MediaBrowser.Common.Configuration;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.IO;
+using Microsoft.Extensions.Logging;
 
 namespace Emby.Server.Implementations.Library;
 
@@ -14,18 +16,22 @@ namespace Emby.Server.Implementations.Library;
 /// </summary>
 public class PathManager : IPathManager
 {
+    private readonly ILogger<PathManager> _logger;
     private readonly IServerConfigurationManager _config;
     private readonly IApplicationPaths _appPaths;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="PathManager"/> class.
     /// </summary>
+    /// <param name="logger">The logger.</param>
     /// <param name="config">The server configuration manager.</param>
     /// <param name="appPaths">The application paths.</param>
     public PathManager(
+        ILogger<PathManager> logger,
         IServerConfigurationManager config,
         IApplicationPaths appPaths)
     {
+        _logger = logger;
         _config = config;
         _appPaths = appPaths;
     }
@@ -35,9 +41,16 @@ public class PathManager : IPathManager
     private string AttachmentCachePath => Path.Combine(_appPaths.DataPath, "attachments");
 
     /// <inheritdoc />
-    public string GetAttachmentPath(string mediaSourceId, string fileName)
+    public string? GetAttachmentPath(string mediaSourceId, string fileName)
     {
-        return Path.Combine(GetAttachmentFolderPath(mediaSourceId), fileName);
+        var safeName = PathHelper.GetSafeLeafFileName(fileName);
+        if (safeName is null)
+        {
+            _logger.LogWarning("Rejecting attachment filename '{FileName}' for MediaSource {MediaSourceId}: not a valid leaf name.", fileName, mediaSourceId);
+            return null;
+        }
+
+        return Path.Combine(GetAttachmentFolderPath(mediaSourceId), safeName);
     }
 
     /// <inheritdoc />

Jellyfin.Server/Migrations/Routines/MoveExtractedFiles.cs

@@ -181,7 +181,9 @@ public class MoveExtractedFiles : IAsyncMigrationRoutine
                 }
             }
 
-            var newAttachmentPath = _pathManager.GetAttachmentPath(itemIdString, attachment.Filename ?? attachmentIndex.ToString(CultureInfo.InvariantCulture));
+            var attachmentIndexName = attachmentIndex.ToString(CultureInfo.InvariantCulture);
+            var newAttachmentPath = _pathManager.GetAttachmentPath(itemIdString, attachment.Filename ?? attachmentIndexName)
+                                     ?? _pathManager.GetAttachmentPath(itemIdString, attachmentIndexName)!;
             if (File.Exists(newAttachmentPath))
             {
                 File.Delete(oldAttachmentPath);

MediaBrowser.Controller/IO/IPathManager.cs

@@ -37,8 +37,8 @@ public interface IPathManager
     /// </summary>
     /// <param name="mediaSourceId">The media source id.</param>
     /// <param name="fileName">The attachmentFileName index.</param>
-    /// <returns>The absolute path.</returns>
-    public string GetAttachmentPath(string mediaSourceId, string fileName);
+    /// <returns>The absolute path, or <c>null</c> if <paramref name="fileName"/> cannot be reduced to a safe leaf name.</returns>
+    public string? GetAttachmentPath(string mediaSourceId, string fileName);
 
     /// <summary>
     /// Gets the path to the attachment folder.

MediaBrowser.MediaEncoding/Attachments/AttachmentExtractor.cs

@@ -6,6 +6,7 @@ using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using AsyncKeyedLock;
+using Jellyfin.Extensions;
 using MediaBrowser.Common.Extensions;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.IO;
@@ -98,9 +99,21 @@ namespace MediaBrowser.MediaEncoding.Attachments
             MediaSourceInfo mediaSource,
             CancellationToken cancellationToken)
         {
-            var shouldExtractOneByOne = mediaSource.MediaAttachments.Any(a => !string.IsNullOrEmpty(a.FileName)
-                                                                              && (a.FileName.Contains('/', StringComparison.OrdinalIgnoreCase) || a.FileName.Contains('\\', StringComparison.OrdinalIgnoreCase)));
-            if (shouldExtractOneByOne && !inputFile.EndsWith(".mks", StringComparison.OrdinalIgnoreCase))
+            var hasUnsafeFileName = mediaSource.MediaAttachments.Any(a => !IsSafeBulkAttachmentFileName(a.FileName));
+            var isMatroskaSubtitles = inputFile.EndsWith(".mks", StringComparison.OrdinalIgnoreCase);
+            if (hasUnsafeFileName && isMatroskaSubtitles)
+            {
+                _logger.LogError(
+                    "Refusing attachment extraction for .mks input {InputFile}: an attachment FileName tag is not a safe leaf name.",
+                    inputFile);
+                throw new InvalidOperationException(
+                    string.Format(
+                        CultureInfo.InvariantCulture,
+                        "Refusing attachment extraction for .mks input {0}: unsafe attachment FileName tag.",
+                        inputFile));
+            }
+
+            if (hasUnsafeFileName)
             {
                 foreach (var attachment in mediaSource.MediaAttachments)
                 {
@@ -241,7 +254,9 @@ namespace MediaBrowser.MediaEncoding.Attachments
             var attachmentFolderPath = _pathManager.GetAttachmentFolderPath(mediaSource.Id);
             using (await _semaphoreLocks.LockAsync(attachmentFolderPath, cancellationToken).ConfigureAwait(false))
             {
-                var attachmentPath = _pathManager.GetAttachmentPath(mediaSource.Id, mediaAttachment.FileName ?? mediaAttachment.Index.ToString(CultureInfo.InvariantCulture));
+                var indexName = mediaAttachment.Index.ToString(CultureInfo.InvariantCulture);
+                var attachmentPath = _pathManager.GetAttachmentPath(mediaSource.Id, mediaAttachment.FileName ?? indexName)
+                                     ?? _pathManager.GetAttachmentPath(mediaSource.Id, indexName)!;
                 if (!File.Exists(attachmentPath))
                 {
                     await ExtractAttachmentInternal(
@@ -341,6 +356,27 @@ namespace MediaBrowser.MediaEncoding.Attachments
             _logger.LogInformation("ffmpeg attachment extraction completed for {InputPath} to {OutputPath}", inputPath, outputPath);
         }
 
+        /// <summary>
+        /// Decides whether an attachment FILENAME tag is safe to feed into ffmpeg's
+        /// bulk <c>-dump_attachment:t ""</c> mode, which writes each attachment using
+        /// its filename tag verbatim relative to the working directory. Anything that
+        /// could escape the working directory - path separators, "..", or empty
+        /// leaves - is rerouted to the one-by-one path where the filename is
+        /// sanitised via <see cref="IPathManager.GetAttachmentPath"/>.
+        /// </summary>
+        private static bool IsSafeBulkAttachmentFileName(string? fileName)
+        {
+            if (string.IsNullOrEmpty(fileName))
+            {
+                return true;
+            }
+
+            // PathHelper.GetSafeLeafFileName collapses to the leaf component;
+            // the bulk path is only safe when the supplied name already _was_
+            // that leaf (no separators, no "."/"..").
+            return PathHelper.GetSafeLeafFileName(fileName) == fileName;
+        }
+
         /// <inheritdoc />
         public void Dispose()
         {

MediaBrowser.Providers/Lyric/LyricManager.cs

@@ -398,7 +398,7 @@ public class LyricManager : ILyricManager
             {
                 var mediaFolderPath = Path.GetFullPath(Path.Combine(audio.ContainingFolderPath, saveFileName));
                 // TODO: Add some error handling to the API user: return BadRequest("Could not save lyric, bad path.");
-                if (mediaFolderPath.StartsWith(audio.ContainingFolderPath, StringComparison.Ordinal))
+                if (PathHelper.IsContainedIn(audio.ContainingFolderPath, mediaFolderPath))
                 {
                     savePaths.Add(mediaFolderPath);
                 }
@@ -407,7 +407,7 @@ public class LyricManager : ILyricManager
             var internalPath = Path.GetFullPath(Path.Combine(audio.GetInternalMetadataPath(), saveFileName));
 
             // TODO: Add some error to the user: return BadRequest("Could not save lyric, bad path.");
-            if (internalPath.StartsWith(audio.GetInternalMetadataPath(), StringComparison.Ordinal))
+            if (PathHelper.IsContainedIn(audio.GetInternalMetadataPath(), internalPath))
             {
                 savePaths.Add(internalPath);
             }

src/Jellyfin.Extensions/PathHelper.cs

@@ -0,0 +1,77 @@
+using System;
+using System.IO;
+
+namespace Jellyfin.Extensions;
+
+/// <summary>
+/// Helpers for safely composing filesystem paths from untrusted input.
+/// </summary>
+/// <remarks>
+/// <see cref="Path.Combine(string, string)"/> has two issues that matter in
+/// any code that joins a trusted directory with an externally-supplied name:
+/// it neither normalises <c>..</c> 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.
+/// </remarks>
+public static class PathHelper
+{
+    /// <summary>
+    /// Reduces a possibly-untrusted file name to a safe leaf-only name with no
+    /// directory components.
+    /// </summary>
+    /// <param name="fileName">The candidate file name.</param>
+    /// <returns>
+    /// The leaf component of <paramref name="fileName"/>, or <c>null</c> if
+    /// the input has no usable leaf (empty, <c>.</c>, or <c>..</c>).
+    /// </returns>
+    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;
+    }
+
+    /// <summary>
+    /// Returns whether <paramref name="candidate"/> resolves to a path that
+    /// equals or is contained inside <paramref name="root"/>.
+    /// </summary>
+    /// <param name="root">The directory the candidate must remain inside.</param>
+    /// <param name="candidate">The candidate absolute or relative path.</param>
+    /// <returns><c>true</c> if the candidate is inside or equal to root; otherwise <c>false</c>.</returns>
+    /// <remarks>
+    /// Both arguments are resolved via <see cref="Path.GetFullPath(string)"/>
+    /// so <c>..</c> segments are collapsed before the comparison. The root is
+    /// compared with a trailing directory separator to prevent prefix
+    /// collisions (e.g. <c>/var/data</c> must not be accepted as a parent of
+    /// <c>/var/dataset</c>).
+    /// </remarks>
+    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);
+    }
+}