Fix GHSA-f47c-m7gr-q92j

This commit is contained in:
Shadowghost
2026-05-22 22:32:48 +02:00
parent 706000cfce
commit 72360ba292
5 changed files with 62 additions and 11 deletions

View File

@@ -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 />

View File

@@ -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);

View File

@@ -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.

View File

@@ -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()
{

View File

@@ -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);
}