From b8c130c73db2c2450e2e49cf3b7bb91dc650e451 Mon Sep 17 00:00:00 2001 From: admin Date: Wed, 17 Dec 2025 19:30:35 -0600 Subject: [PATCH] fix(security): patch SQL injection, path traversal, command injection --- src/NzbDrone.Common/ArchiveService.cs | 13 ++++++++++++- src/NzbDrone.Common/Processes/ProcessProvider.cs | 6 +++--- .../Housekeeping/Housekeepers/CleanupUnusedTags.cs | 8 +++----- .../Frontend/Mappers/MediaCoverMapper.cs | 10 +++++++++- .../Frontend/Mappers/StaticResourceMapper.cs | 12 +++++++++++- 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/NzbDrone.Common/ArchiveService.cs b/src/NzbDrone.Common/ArchiveService.cs index d420bbbc0a..0b2c068e98 100644 --- a/src/NzbDrone.Common/ArchiveService.cs +++ b/src/NzbDrone.Common/ArchiveService.cs @@ -69,6 +69,8 @@ private void ExtractZip(string compressedFile, string destination) throw new IOException(string.Format("File {0} failed archive validation.", compressedFile)); } + var destinationFullPath = Path.GetFullPath(destination); + foreach (ZipEntry zipEntry in zipFile) { if (!zipEntry.IsFile) @@ -85,7 +87,16 @@ private void ExtractZip(string compressedFile, string destination) var zipStream = zipFile.GetInputStream(zipEntry); // Manipulate the output filename here as desired. - var fullZipToPath = Path.Combine(destination, entryFileName); + var fullZipToPath = Path.GetFullPath(Path.Combine(destination, entryFileName)); + + // Prevent path traversal attacks - ensure extracted path is within destination + if (!fullZipToPath.StartsWith(destinationFullPath + Path.DirectorySeparatorChar) && + !fullZipToPath.Equals(destinationFullPath, StringComparison.Ordinal)) + { + _logger.Warn("Skipping zip entry with path traversal attempt: {0}", entryFileName); + continue; + } + var directoryName = Path.GetDirectoryName(fullZipToPath); if (directoryName.Length > 0) { diff --git a/src/NzbDrone.Common/Processes/ProcessProvider.cs b/src/NzbDrone.Common/Processes/ProcessProvider.cs index 8dc5e9f836..ed6c598a58 100644 --- a/src/NzbDrone.Common/Processes/ProcessProvider.cs +++ b/src/NzbDrone.Common/Processes/ProcessProvider.cs @@ -356,17 +356,17 @@ private List GetProcessesByName(string name) { if (OsInfo.IsWindows && path.EndsWith(".bat", StringComparison.InvariantCultureIgnoreCase)) { - return ("cmd.exe", $"/c {path} {args}"); + return ("cmd.exe", $"/c \"{path}\" {args}"); } if (OsInfo.IsWindows && path.EndsWith(".ps1", StringComparison.InvariantCultureIgnoreCase)) { - return ("powershell.exe", $"-ExecutionPolicy Bypass -NoProfile -File {path} {args}"); + return ("powershell.exe", $"-ExecutionPolicy Bypass -NoProfile -File \"{path}\" {args}"); } if (OsInfo.IsWindows && path.EndsWith(".py", StringComparison.InvariantCultureIgnoreCase)) { - return ("python.exe", $"{path} {args}"); + return ("python.exe", $"\"{path}\" {args}"); } return (path, args); diff --git a/src/NzbDrone.Core/Housekeeping/Housekeepers/CleanupUnusedTags.cs b/src/NzbDrone.Core/Housekeeping/Housekeepers/CleanupUnusedTags.cs index a259dd8af7..c79edebdb3 100644 --- a/src/NzbDrone.Core/Housekeeping/Housekeepers/CleanupUnusedTags.cs +++ b/src/NzbDrone.Core/Housekeeping/Housekeepers/CleanupUnusedTags.cs @@ -31,19 +31,17 @@ public void Clean() .SelectMany(v => GetUsedTags(v, mapper)) .Concat(GetAutoTaggingTagSpecificationTags(mapper)) .Distinct() - .ToList(); + .ToArray(); if (usedTags.Any()) { - var usedTagsList = usedTags.Select(d => d.ToString()).Join(","); - if (_database.DatabaseType == DatabaseType.PostgreSQL) { - mapper.Execute($"DELETE FROM \"Tags\" WHERE NOT \"Id\" = ANY (\'{{{usedTagsList}}}\'::int[])"); + mapper.Execute("DELETE FROM \"Tags\" WHERE NOT \"Id\" = ANY (@UsedTags)", new { UsedTags = usedTags }); } else { - mapper.Execute($"DELETE FROM \"Tags\" WHERE NOT \"Id\" IN ({usedTagsList})"); + mapper.Execute("DELETE FROM \"Tags\" WHERE \"Id\" NOT IN @UsedTags", new { UsedTags = usedTags }); } } else diff --git a/src/Radarr.Http/Frontend/Mappers/MediaCoverMapper.cs b/src/Radarr.Http/Frontend/Mappers/MediaCoverMapper.cs index 8522edf625..5389558c76 100644 --- a/src/Radarr.Http/Frontend/Mappers/MediaCoverMapper.cs +++ b/src/Radarr.Http/Frontend/Mappers/MediaCoverMapper.cs @@ -27,7 +27,15 @@ public override string Map(string resourceUrl) var path = resourceUrl.Replace('/', Path.DirectorySeparatorChar); path = path.Trim(Path.DirectorySeparatorChar); - var resourcePath = Path.Combine(_appFolderInfo.GetAppDataPath(), path); + var basePath = Path.GetFullPath(_appFolderInfo.GetAppDataPath()); + var resourcePath = Path.GetFullPath(Path.Combine(basePath, path)); + + // Prevent path traversal attacks - ensure path stays within AppData folder + if (!resourcePath.StartsWith(basePath + Path.DirectorySeparatorChar) && + !resourcePath.Equals(basePath, StringComparison.Ordinal)) + { + return null; + } if (!_diskProvider.FileExists(resourcePath) || _diskProvider.GetFileSize(resourcePath) == 0) { diff --git a/src/Radarr.Http/Frontend/Mappers/StaticResourceMapper.cs b/src/Radarr.Http/Frontend/Mappers/StaticResourceMapper.cs index b9dcd34d9b..5b7d83b1d2 100644 --- a/src/Radarr.Http/Frontend/Mappers/StaticResourceMapper.cs +++ b/src/Radarr.Http/Frontend/Mappers/StaticResourceMapper.cs @@ -23,7 +23,17 @@ public override string Map(string resourceUrl) var path = resourceUrl.Replace('/', Path.DirectorySeparatorChar); path = path.Trim(Path.DirectorySeparatorChar); - return Path.Combine(_appFolderInfo.StartUpFolder, _configFileProvider.UiFolder, path); + var basePath = Path.GetFullPath(Path.Combine(_appFolderInfo.StartUpFolder, _configFileProvider.UiFolder)); + var fullPath = Path.GetFullPath(Path.Combine(basePath, path)); + + // Prevent path traversal attacks - ensure path stays within UI folder + if (!fullPath.StartsWith(basePath + Path.DirectorySeparatorChar) && + !fullPath.Equals(basePath, StringComparison.Ordinal)) + { + return null; + } + + return fullPath; } public override bool CanHandle(string resourceUrl)