Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 39 additions & 4 deletions Downloads/DownloadQueueController.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Playnite.SDK;
using Playnite.SDK.Plugins;
using RomM.Games;
using SharpCompress.Archives;
using SharpCompress.Common;
using System;
Expand Down Expand Up @@ -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(@"..\"))
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
25 changes: 22 additions & 3 deletions Games/RomMImport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment thread
gantoine marked this conversation as resolved.

var status = _plugin.Playnite.Database.CompletionStatuses.Get(StatusID);
var completionStatusProperty = status != null ? new MetadataNameProperty(status.Name) : null;
Expand Down
7 changes: 5 additions & 2 deletions Games/RomMInstallController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
54 changes: 52 additions & 2 deletions Games/RomMInstallPaths.cs
Original file line number Diff line number Diff line change
@@ -1,19 +1,69 @@
using System;
using System.IO;
using System.Linq;

namespace RomM.Games
{
// Derives a ROM's install directory and playable path. These MUST come from the actual ROM file
// 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;
}

// <root>/<file name without extension>
public static string InstallDir(string rootInstallDir, string fileName)
=> Path.Combine(rootInstallDir, Path.GetFileNameWithoutExtension(fileName));
=> Path.Combine(rootInstallDir, Path.GetFileNameWithoutExtension(Contained(fileName)));

// <root>/<folder name> when folderName is set, otherwise <root>/<file name without extension>.
public static string InstallDir(string rootInstallDir, string folderName, string fileName)
=> string.IsNullOrEmpty(folderName)
? InstallDir(rootInstallDir, fileName)
: Path.Combine(rootInstallDir, Contained(folderName));

// <root>/<file name without extension>/<file name>
public static string GamePath(string rootInstallDir, string fileName)
=> Path.Combine(InstallDir(rootInstallDir, fileName), fileName);
=> Path.Combine(InstallDir(rootInstallDir, fileName), Contained(fileName));

// <install dir>/<file name>, using the folder-aware install dir.
public static string GamePath(string rootInstallDir, string folderName, string fileName)
=> Path.Combine(InstallDir(rootInstallDir, folderName, fileName), Contained(fileName));
}
}
25 changes: 25 additions & 0 deletions Games/RomMRevisionFactory.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using RomM.Models.RomM.Rom;

Expand All @@ -20,6 +22,24 @@ public static RomMFile SelectPrimaryFile(IList<RomMFile> 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.
Expand All @@ -39,13 +59,18 @@ 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}");
}
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}");
}

Expand Down
1 change: 1 addition & 0 deletions Models/RomM/Rom/GameInstallInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
7 changes: 7 additions & 0 deletions Models/RomM/Rom/RomMRomLocal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
72 changes: 72 additions & 0 deletions RomM.Tests/RomMInstallPathsTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.IO;
using RomM.Games;
using Xunit;
Expand Down Expand Up @@ -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<ArgumentException>(() => RomMInstallPaths.InstallDir(Root, hostile, "game.gba"));
Assert.Throws<ArgumentException>(() => RomMInstallPaths.GamePath(Root, "folder", hostile));
Assert.Throws<ArgumentException>(() => 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<ArgumentException>(
() => 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));
}
}
}
Loading
Loading