From 5104497331c0519c551e1af6b3999f0da0d65058 Mon Sep 17 00:00:00 2001 From: David Federman Date: Tue, 2 Jun 2026 23:12:50 -0700 Subject: Reject unsafe plugin package names in installer --- .../Updates/InstallationManager.cs | 43 ++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 'Emby.Server.Implementations/Updates') diff --git a/Emby.Server.Implementations/Updates/InstallationManager.cs b/Emby.Server.Implementations/Updates/InstallationManager.cs index ef53e3b326..c8a2d98bf4 100644 --- a/Emby.Server.Implementations/Updates/InstallationManager.cs +++ b/Emby.Server.Implementations/Updates/InstallationManager.cs @@ -521,9 +521,27 @@ namespace Emby.Server.Implementations.Updates return; } + if (!IsValidPackageDirectoryName(package.Name)) + { + _logger.LogError("Refusing to install package with invalid name {PackageName}.", package.Name); + throw new InvalidDataException($"Plugin package name '{package.Name}' is not a valid directory name."); + } + // Always override the passed-in target (which is a file) and figure it out again string targetDir = Path.Combine(_appPaths.PluginsPath, package.Name); + var pluginsRoot = Path.TrimEndingDirectorySeparator(Path.GetFullPath(_appPaths.PluginsPath)); + var resolvedTarget = Path.GetFullPath(targetDir); + if (!resolvedTarget.StartsWith(pluginsRoot + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogError( + "Refusing to install package {PackageName}: resolved target {Resolved} is outside plugins directory {Root}.", + package.Name, + resolvedTarget, + pluginsRoot); + throw new InvalidDataException($"Plugin package name '{package.Name}' resolves outside the plugins directory."); + } + using var response = await _httpClientFactory.CreateClient(NamedClient.Default) .GetAsync(new Uri(package.SourceUrl), cancellationToken).ConfigureAwait(false); response.EnsureSuccessStatusCode(); @@ -572,6 +590,31 @@ namespace Emby.Server.Implementations.Updates _pluginManager.ImportPluginFrom(targetDir); } + private static bool IsValidPackageDirectoryName(string? name) + { + if (string.IsNullOrWhiteSpace(name)) + { + return false; + } + + if (name.Equals(".", StringComparison.Ordinal) || name.Equals("..", StringComparison.Ordinal)) + { + return false; + } + + if (name.Contains('/', StringComparison.Ordinal) || name.Contains('\\', StringComparison.Ordinal)) + { + return false; + } + + if (name.AsSpan().IndexOfAny(Path.GetInvalidFileNameChars()) >= 0) + { + return false; + } + + return true; + } + private async Task InstallPackageInternal(InstallationInfo package, CancellationToken cancellationToken) { LocalPlugin? plugin = _pluginManager.Plugins.FirstOrDefault(p => p.Id.Equals(package.Id) && p.Version.Equals(package.Version)) -- cgit v1.2.3 From 26a149a970ad1f88fbd6e1676a5098e4a63531fe Mon Sep 17 00:00:00 2001 From: David Federman Date: Wed, 3 Jun 2026 08:04:39 -0700 Subject: Address PR comment --- Emby.Server.Implementations/Updates/InstallationManager.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'Emby.Server.Implementations/Updates') diff --git a/Emby.Server.Implementations/Updates/InstallationManager.cs b/Emby.Server.Implementations/Updates/InstallationManager.cs index c8a2d98bf4..110c388fbe 100644 --- a/Emby.Server.Implementations/Updates/InstallationManager.cs +++ b/Emby.Server.Implementations/Updates/InstallationManager.cs @@ -32,6 +32,8 @@ namespace Emby.Server.Implementations.Updates /// public class InstallationManager : IInstallationManager { + private static readonly char[] InvlidPackageNameChars = [.. Path.GetInvalidFileNameChars(), '/', '\\']; + /// /// The logger. /// @@ -602,12 +604,7 @@ namespace Emby.Server.Implementations.Updates return false; } - if (name.Contains('/', StringComparison.Ordinal) || name.Contains('\\', StringComparison.Ordinal)) - { - return false; - } - - if (name.AsSpan().IndexOfAny(Path.GetInvalidFileNameChars()) >= 0) + if (name.AsSpan().IndexOfAny(InvlidPackageNameChars) >= 0) { return false; } -- cgit v1.2.3 From 0ed27bad65aa48c4c39c74493a90c3c81795d5ab Mon Sep 17 00:00:00 2001 From: David Federman Date: Sat, 6 Jun 2026 21:55:30 -0700 Subject: Address PR comment --- Emby.Server.Implementations/Updates/InstallationManager.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'Emby.Server.Implementations/Updates') diff --git a/Emby.Server.Implementations/Updates/InstallationManager.cs b/Emby.Server.Implementations/Updates/InstallationManager.cs index 110c388fbe..6a60f7f5f6 100644 --- a/Emby.Server.Implementations/Updates/InstallationManager.cs +++ b/Emby.Server.Implementations/Updates/InstallationManager.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; @@ -32,7 +33,7 @@ namespace Emby.Server.Implementations.Updates /// public class InstallationManager : IInstallationManager { - private static readonly char[] InvlidPackageNameChars = [.. Path.GetInvalidFileNameChars(), '/', '\\']; + private static readonly SearchValues InvalidPackageNameChars = SearchValues.Create([.. Path.GetInvalidFileNameChars(), '/', '\\']); /// /// The logger. @@ -604,7 +605,7 @@ namespace Emby.Server.Implementations.Updates return false; } - if (name.AsSpan().IndexOfAny(InvlidPackageNameChars) >= 0) + if (name.IndexOfAny(InvalidPackageNameChars) >= 0) { return false; } -- cgit v1.2.3