diff --git a/Downloads/DownloadQueueController.cs b/Downloads/DownloadQueueController.cs index c68c579..f186f45 100644 --- a/Downloads/DownloadQueueController.cs +++ b/Downloads/DownloadQueueController.cs @@ -1,5 +1,6 @@ using Playnite.SDK; using Playnite.SDK.Plugins; +using RomM.Games; using SharpCompress.Archives; using SharpCompress.Common; using System; @@ -213,6 +214,32 @@ private static bool IsFileCompressed(string filePath) } + // 7-Zip extracts into the output dir and drops absolute paths and ".." components unless -spf + // is passed (we never pass it), but the entry names are checked up front anyway so both + // extraction paths refuse traversal the same way. Formats SharpCompress cannot open fall back + // to 7-Zip's own handling rather than failing a download that used to work. + private void EnsureEntriesContained(string archivePath, string installDir) + { + try + { + using (var archive = ArchiveFactory.Open(archivePath)) + { + foreach (var entry in archive.Entries.Where(e => !e.IsDirectory)) + { + RomMInstallPaths.ResolveWithin(installDir, entry.Key); + } + } + } + catch (ArgumentException) + { + throw; + } + catch (Exception ex) + { + Logger.Warn($"Could not inspect {archivePath} before extraction: {ex.Message}"); + } + } + private void ExtractArchiveWith7z(string pathTo7z, string archivePath, string installDir, DownloadQueueItem item, CancellationToken ct) { if (archivePath == null || archivePath.Contains("../") || archivePath.Contains(@"..\")) @@ -224,6 +251,8 @@ private void ExtractArchiveWith7z(string pathTo7z, string archivePath, string in throw new ArgumentException("Invalid install directory path"); } + EnsureEntriesContained(archivePath, installDir); + ProcessStartInfo startInfo = new ProcessStartInfo { FileName = pathTo7z, @@ -256,11 +285,17 @@ private void ExtractArchiveWithEntryProgress(string archivePath, string installD { ct.ThrowIfCancellationRequested(); - entry.WriteToDirectory(installDir, new ExtractionOptions + // Entry names come from the downloaded archive, so they are untrusted: handing a + // "../" or rooted key to ExtractFullPath would write outside installDir. Resolve + // and verify each destination, then copy the entry there ourselves. + var destination = RomMInstallPaths.ResolveWithin(installDir, entry.Key); + Directory.CreateDirectory(Path.GetDirectoryName(destination)); + + using (var entryStream = entry.OpenEntryStream()) + using (var file = File.Create(destination)) { - ExtractFullPath = true, - Overwrite = true - }); + entryStream.CopyTo(file); + } done++; item.SetProgress(done, total, false); diff --git a/Games/RomMImport.cs b/Games/RomMImport.cs index a1b352f..bc58364 100644 --- a/Games/RomMImport.cs +++ b/Games/RomMImport.cs @@ -189,10 +189,29 @@ private Game ImportGame(RomMRom ROM, Guid StatusID) // Paths must be derived from the actual ROM file (what RomMInstallController downloads), // not the display Name. Using Name drops the extension and can include characters that // don't match the installed file, breaking IsInstalled detection and the play path. + // + // For folder-based ROMs we point at a real file inside the ROM's folder (fs_name): + // - nested single file: the one file in the folder, + // - multiple files: the primary file (the download descriptor's FileName is the folder + // name / archive base, which is not itself a real file, so use the primary file here). var baseRevision = BuildRevision(ROM); - var fileName = !string.IsNullOrEmpty(baseRevision?.FileName) ? baseRevision.FileName : ROM.Name; - var gameInstallDir = RomMInstallPaths.InstallDir(rootInstallDir, fileName); - var pathToGame = RomMInstallPaths.GamePath(rootInstallDir, fileName); + var folderName = baseRevision?.FolderName; + var playableFile = ROM.HasMultipleFiles + ? RomMRevisionFactory.RelativeFilePath(RomMRevisionFactory.SelectPrimaryFile(ROM.Files), folderName) + : baseRevision?.FileName; + // With no file list, fs_name still beats the extensionless display Name. + var fileName = !string.IsNullOrEmpty(playableFile) ? playableFile + : !string.IsNullOrEmpty(ROM.FileName) ? ROM.FileName + : ROM.Name; + // Skip the ROM rather than letting the throw from RomMInstallPaths abort the whole platform. + if (!RomMInstallPaths.IsContained(folderName) || !RomMInstallPaths.IsContained(fileName)) + { + _plugin.Logger.Error($"[Importer] RomM ID {ROM.Id} has a path outside the install root: {folderName} / {fileName}"); + return null; + } + + var gameInstallDir = RomMInstallPaths.InstallDir(rootInstallDir, folderName, fileName); + var pathToGame = RomMInstallPaths.GamePath(rootInstallDir, folderName, fileName); var status = _plugin.Playnite.Database.CompletionStatuses.Get(StatusID); var completionStatusProperty = status != null ? new MetadataNameProperty(status.Name) : null; diff --git a/Games/RomMInstallController.cs b/Games/RomMInstallController.cs index 8cc37ba..5ee2b07 100644 --- a/Games/RomMInstallController.cs +++ b/Games/RomMInstallController.cs @@ -41,8 +41,11 @@ public override void Install(InstallActionArgs args) var dstPath = _gameData.Mapping?.DestinationPathResolved ?? throw new Exception("Mapped emulator data cannot be found, try removing and re-adding."); - // Paths (same as before) - var installDir = Path.Combine(dstPath, Path.GetFileNameWithoutExtension(_gameData.FileName)); + // Install dir mirrors RomM's on-disk layout: folder-based ROMs (nested single / multiple + // files) install into the ROM's folder (fs_name); simple single files fall back to a + // folder derived from the file name. Must match the path computed at import time so + // IsInstalled detection lines up. + var installDir = RomMInstallPaths.InstallDir(dstPath, _gameData.FolderName, _gameData.FileName); // If RomM indicates multiple files, we download as an archive name (zip) into the install folder. // Otherwise we download the single ROM file. diff --git a/Games/RomMInstallPaths.cs b/Games/RomMInstallPaths.cs index 87a7bb3..589c805 100644 --- a/Games/RomMInstallPaths.cs +++ b/Games/RomMInstallPaths.cs @@ -1,4 +1,6 @@ +using System; using System.IO; +using System.Linq; namespace RomM.Games { @@ -6,14 +8,62 @@ namespace RomM.Games // name (what gets downloaded), not the display name: using the display name drops the extension // and can include characters that don't match the installed file, breaking IsInstalled detection // and the play path. + // + // For folder-based ROMs (nested single file / multiple files) a non-null folderName (fs_name) + // pins the directory to the ROM's actual folder on the RomM filesystem, instead of deriving it + // from the download file name — the file name can carry region tags and an extension that the + // containing folder does not (e.g. file "Game (Europe).zip" inside folder "Game"). internal static class RomMInstallPaths { + // fs_name and file names come straight from the server, so they are untrusted. A rooted value + // ("/tmp", @"C:\x", @"\x") makes Path.Combine discard rootInstallDir and ".." walks back out + // of it — either would let the download and archive extraction write outside the configured + // mapping. Nested relative paths (a primary file inside a subfolder) stay allowed. + // Rooting is checked by hand rather than via Path.IsPathRooted so a Windows-rooted value is + // still rejected when this runs on another platform (e.g. the test host). + public static bool IsContained(string path) + => string.IsNullOrEmpty(path) + || (path[0] != '/' + && path[0] != '\\' + && path.IndexOf(':') < 0 + && !path.Split('/', '\\').Any(segment => segment == "..")); + + private static string Contained(string path) + => IsContained(path) ? path : throw new ArgumentException($"Path from RomM escapes the install root: {path}"); + + // Resolves an untrusted relative path against a trusted root, throwing unless the result stays + // inside it. Archive entry names are attacker-controlled too, so extraction resolves every + // destination through here instead of handing raw keys to SharpCompress' ExtractFullPath. + public static string ResolveWithin(string root, string relativePath) + { + if (string.IsNullOrEmpty(relativePath)) + throw new ArgumentException("Archive entry has no name, refusing to extract it."); + + var fullRoot = Path.GetFullPath(root).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + var destination = Path.GetFullPath(Path.Combine(fullRoot, Contained(relativePath))); + + if (!destination.StartsWith(fullRoot + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase)) + throw new ArgumentException($"Path escapes the install directory: {relativePath}"); + + return destination; + } + // / public static string InstallDir(string rootInstallDir, string fileName) - => Path.Combine(rootInstallDir, Path.GetFileNameWithoutExtension(fileName)); + => Path.Combine(rootInstallDir, Path.GetFileNameWithoutExtension(Contained(fileName))); + + // / when folderName is set, otherwise /. + public static string InstallDir(string rootInstallDir, string folderName, string fileName) + => string.IsNullOrEmpty(folderName) + ? InstallDir(rootInstallDir, fileName) + : Path.Combine(rootInstallDir, Contained(folderName)); // // public static string GamePath(string rootInstallDir, string fileName) - => Path.Combine(InstallDir(rootInstallDir, fileName), fileName); + => Path.Combine(InstallDir(rootInstallDir, fileName), Contained(fileName)); + + // /, using the folder-aware install dir. + public static string GamePath(string rootInstallDir, string folderName, string fileName) + => Path.Combine(InstallDir(rootInstallDir, folderName, fileName), Contained(fileName)); } } diff --git a/Games/RomMRevisionFactory.cs b/Games/RomMRevisionFactory.cs index b4d205a..7a39765 100644 --- a/Games/RomMRevisionFactory.cs +++ b/Games/RomMRevisionFactory.cs @@ -1,4 +1,6 @@ +using System; using System.Collections.Generic; +using System.IO; using System.Linq; using RomM.Models.RomM.Rom; @@ -20,6 +22,24 @@ public static RomMFile SelectPrimaryFile(IList files) return files.FirstOrDefault(); } + // A file's path relative to the ROM folder (fs_name). Extraction preserves subdirectories, so + // a file below another directory needs "sub/file.bin", not just "file.bin". Falls back to the + // leaf name when the folder is not part of the full path. + public static string RelativeFilePath(RomMFile file, string folderName) + { + if (file == null) + return null; + + var segments = (file.FullPath ?? string.Empty).Split('/'); + var folderIndex = string.IsNullOrEmpty(folderName) + ? -1 + : Array.FindLastIndex(segments, s => s.Equals(folderName, StringComparison.OrdinalIgnoreCase)); + + return folderIndex >= 0 && folderIndex < segments.Length - 1 + ? string.Join(Path.DirectorySeparatorChar.ToString(), segments.Skip(folderIndex + 1)) + : file.FileName; + } + // Returns null when a single-file ROM has no resolvable file. Single files use the 4.9 // /files/content endpoint when a file id is present, falling back to the rom-level endpoint // (so we never emit "api/roms//files/content/..."); multi-file ROMs download the whole archive. @@ -39,6 +59,9 @@ public static RomMRevision Build(RomMRom rom, string romMHost) return null; revision.FileName = romfile.FileName; + // A nested single file lives inside a folder named after the ROM (fs_name); a simple + // single file sits directly in the platform folder and has no wrapping folder. + revision.FolderName = rom.HasNestedSingleFile ? rom.FileName : null; revision.DownloadURL = romfile.Id.HasValue ? RomMUrl.Combine(romMHost, $"api/roms/{romfile.Id}/files/content/{romfile.FileName}") : RomMUrl.Combine(romMHost, $"api/roms/{rom.Id}/content/{romfile.FileName}"); @@ -46,6 +69,8 @@ public static RomMRevision Build(RomMRom rom, string romMHost) else { revision.FileName = rom.FileName; + // Multi-file ROMs are always stored in a folder named after the ROM (fs_name). + revision.FolderName = rom.FileName; revision.DownloadURL = RomMUrl.Combine(romMHost, $"api/roms/{rom.Id}/content/{rom.FileName}"); } diff --git a/Models/RomM/Rom/GameInstallInfo.cs b/Models/RomM/Rom/GameInstallInfo.cs index 6097de1..abdd76f 100644 --- a/Models/RomM/Rom/GameInstallInfo.cs +++ b/Models/RomM/Rom/GameInstallInfo.cs @@ -8,6 +8,7 @@ public struct GameInstallInfo { public int Id { get; set; } public string FileName { get; set; } + public string FolderName { get; set; } public bool HasMultipleFiles { get; set; } public string DownloadURL { get; set; } public EmulatorMapping Mapping { get; set; } diff --git a/Models/RomM/Rom/RomMRomLocal.cs b/Models/RomM/Rom/RomMRomLocal.cs index 4d80e35..95c393d 100644 --- a/Models/RomM/Rom/RomMRomLocal.cs +++ b/Models/RomM/Rom/RomMRomLocal.cs @@ -14,6 +14,13 @@ public class RomMRevision { public int Id { get; set; } public string FileName { get; set; } + + // The ROM's folder on the RomM filesystem (fs_name) for folder-based ROMs (nested single file + // or multiple files). Null/empty for a "simple" single file that lives directly in the platform + // folder. Install paths use this so they mirror RomM's on-disk layout instead of being derived + // from the download file name (which can carry region tags / an extension the folder doesn't). + public string FolderName { get; set; } + public bool HasMultipleFiles { get; set; } public string DownloadURL { get; set; } public bool IsSelected { get; set; } diff --git a/RomM.Tests/RomMInstallPathsTests.cs b/RomM.Tests/RomMInstallPathsTests.cs index dd27c87..476e12b 100644 --- a/RomM.Tests/RomMInstallPathsTests.cs +++ b/RomM.Tests/RomMInstallPathsTests.cs @@ -1,3 +1,4 @@ +using System; using System.IO; using RomM.Games; using Xunit; @@ -31,5 +32,76 @@ public void Handles_filename_without_extension() Assert.Equal(Path.Combine(Root, "game"), RomMInstallPaths.InstallDir(Root, "game")); Assert.Equal(Path.Combine(Root, "game", "game"), RomMInstallPaths.GamePath(Root, "game")); } + + [Fact] + public void Folder_name_pins_install_dir_to_the_rom_folder() + { + // Nested single file: folder is the ROM name (fs_name), file carries a region tag. + const string folder = "All-Star Baseball '99"; + const string file = "All-Star Baseball '99 (Europe).zip"; + + Assert.Equal( + Path.Combine(Root, folder), + RomMInstallPaths.InstallDir(Root, folder, file)); + Assert.Equal( + Path.Combine(Root, folder, file), + RomMInstallPaths.GamePath(Root, folder, file)); + } + + [Theory] + [InlineData("/etc")] + [InlineData(@"C:\Windows")] + [InlineData(@"\Windows")] + [InlineData("../../elsewhere")] + [InlineData(@"folder\..\..\elsewhere")] + public void Rejects_paths_that_escape_the_install_root(string hostile) + { + Assert.False(RomMInstallPaths.IsContained(hostile)); + Assert.Throws(() => RomMInstallPaths.InstallDir(Root, hostile, "game.gba")); + Assert.Throws(() => RomMInstallPaths.GamePath(Root, "folder", hostile)); + Assert.Throws(() => RomMInstallPaths.GamePath(Root, hostile)); + } + + [Fact] + public void ResolveWithin_keeps_archive_entries_under_the_install_dir() + { + var installDir = Path.Combine(Root, "Final Fantasy VII"); + + Assert.Equal( + Path.GetFullPath(Path.Combine(installDir, "Disc 1", "disc1.bin")), + RomMInstallPaths.ResolveWithin(installDir, "Disc 1/disc1.bin")); + } + + [Theory] + [InlineData("../evil.exe")] + [InlineData("Disc 1/../../evil.exe")] + [InlineData("/etc/evil")] + [InlineData(@"C:\Windows\evil.exe")] + [InlineData("")] + public void ResolveWithin_rejects_entries_outside_the_install_dir(string entryKey) + { + Assert.Throws( + () => RomMInstallPaths.ResolveWithin(Path.Combine(Root, "Final Fantasy VII"), entryKey)); + } + + [Fact] + public void Allows_nested_relative_file_paths() + { + Assert.True(RomMInstallPaths.IsContained(Path.Combine("Disc 1", "disc1.bin"))); + Assert.True(RomMInstallPaths.IsContained("Advance Wars (USA).gba")); + } + + [Fact] + public void Null_or_empty_folder_name_falls_back_to_filename_derived_dir() + { + const string file = "Advance Wars (USA).gba"; + + Assert.Equal( + RomMInstallPaths.InstallDir(Root, file), + RomMInstallPaths.InstallDir(Root, null, file)); + Assert.Equal( + RomMInstallPaths.GamePath(Root, file), + RomMInstallPaths.GamePath(Root, "", file)); + } } } diff --git a/RomM.Tests/RomMRevisionFactoryTests.cs b/RomM.Tests/RomMRevisionFactoryTests.cs index dae2980..390951a 100644 --- a/RomM.Tests/RomMRevisionFactoryTests.cs +++ b/RomM.Tests/RomMRevisionFactoryTests.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.IO; using RomM.Games; using RomM.Models.RomM.Rom; using Xunit; @@ -28,6 +29,25 @@ public void SelectPrimaryFile_null_when_empty_or_null() Assert.Null(RomMRevisionFactory.SelectPrimaryFile(null)); } + [Fact] + public void RelativeFilePath_keeps_path_below_the_rom_folder() + { + var file = new RomMFile { FileName = "disc1.bin", FullPath = "roms/ps1/Final Fantasy VII/Disc 1/disc1.bin" }; + + Assert.Equal(Path.Combine("Disc 1", "disc1.bin"), + RomMRevisionFactory.RelativeFilePath(file, "Final Fantasy VII")); + } + + [Fact] + public void RelativeFilePath_falls_back_to_leaf_name() + { + var file = new RomMFile { FileName = "disc1.bin", FullPath = "roms/ps1/Other Folder/disc1.bin" }; + + Assert.Equal("disc1.bin", RomMRevisionFactory.RelativeFilePath(file, "Final Fantasy VII")); + Assert.Equal("disc1.bin", RomMRevisionFactory.RelativeFilePath(file, null)); + Assert.Null(RomMRevisionFactory.RelativeFilePath(null, "Final Fantasy VII")); + } + [Fact] public void Single_file_with_id_uses_files_content_endpoint() { @@ -46,6 +66,61 @@ public void Single_file_with_id_uses_files_content_endpoint() Assert.Equal(Host + "/api/roms/7/files/content/game.gba", rev.DownloadURL); } + [Fact] + public void Simple_single_file_has_no_folder_name() + { + var rom = new RomMRom + { + Id = 32, + HasSimpleSingleFile = true, + HasMultipleFiles = false, + FileName = "game.gba", + Files = new List { new RomMFile { Id = 7, FileName = "game.gba", FullPath = "game.gba" } }, + }; + + var rev = RomMRevisionFactory.Build(rom, Host); + + Assert.Null(rev.FolderName); + } + + [Fact] + public void Nested_single_file_folder_name_is_the_rom_folder() + { + var rom = new RomMRom + { + Id = 33, + HasNestedSingleFile = true, + HasMultipleFiles = false, + FileName = "All-Star Baseball '99", + Files = new List + { + new RomMFile { Id = 8, FileName = "All-Star Baseball '99 (Europe).zip", FullPath = "All-Star Baseball '99/All-Star Baseball '99 (Europe).zip" }, + }, + }; + + var rev = RomMRevisionFactory.Build(rom, Host); + + // File is the real inner file; folder is fs_name (the ROM folder). + Assert.Equal("All-Star Baseball '99 (Europe).zip", rev.FileName); + Assert.Equal("All-Star Baseball '99", rev.FolderName); + } + + [Fact] + public void Multi_file_folder_name_is_the_rom_folder() + { + var rom = new RomMRom + { + Id = 40, + HasMultipleFiles = true, + FileName = "1080 TenEighty Snowboarding", + Files = new List(), + }; + + var rev = RomMRevisionFactory.Build(rom, Host); + + Assert.Equal("1080 TenEighty Snowboarding", rev.FolderName); + } + [Fact] public void Single_file_without_id_falls_back_to_rom_endpoint() { diff --git a/RomM.cs b/RomM.cs index 81780fb..dacd444 100644 --- a/RomM.cs +++ b/RomM.cs @@ -418,6 +418,7 @@ public override IEnumerable GetInstallActions(GetInstallActio { Id = gameData.ROMVersions[0].Id, FileName = gameData.ROMVersions[0].FileName, + FolderName = gameData.ROMVersions[0].FolderName, HasMultipleFiles = gameData.ROMVersions[0].HasMultipleFiles, DownloadURL = gameData.ROMVersions[0].DownloadURL, Mapping = Settings.Mappings.FirstOrDefault(x => x.MappingId == gameData.MappingID) @@ -466,6 +467,7 @@ public override IEnumerable GetInstallActions(GetInstallActio var selectedrevision = VersionSelectorControl.RomVersions.First(x => x.IsSelected); romData.Id = selectedrevision.Id; romData.FileName = selectedrevision.FileName; + romData.FolderName = selectedrevision.FolderName; romData.HasMultipleFiles = selectedrevision.HasMultipleFiles; romData.DownloadURL = selectedrevision.DownloadURL;