From c5d908cc69ce3d5f8522d57dc4c77c0ea7a77591 Mon Sep 17 00:00:00 2001 From: zeromus Date: Wed, 20 Jan 2021 15:20:55 -0500 Subject: [PATCH] HawkFile: limit attempts to dearchive to files with extension .zip, .7z, .rar. fixes #2587 --- src/BizHawk.Client.Common/RomLoader.cs | 9 ++-- .../SharpCompressDearchivalMethod.cs | 11 +++-- src/BizHawk.Common/HawkFile/HawkFile.cs | 42 +++++++++---------- .../HawkFile/IFileDearchivalMethod.cs | 4 ++ 4 files changed, 35 insertions(+), 31 deletions(-) diff --git a/src/BizHawk.Client.Common/RomLoader.cs b/src/BizHawk.Client.Common/RomLoader.cs index 7c9dde5edb..e5fd55ba45 100644 --- a/src/BizHawk.Client.Common/RomLoader.cs +++ b/src/BizHawk.Client.Common/RomLoader.cs @@ -604,12 +604,9 @@ namespace BizHawk.Client.Common return false; } - using var file = new HawkFile( - path, - nonArchiveExtensions: OpenAdvanced is OpenAdvanced_MAME - ? new[] { ".zip", ".7z" } // MAME uses these extensions for arcade ROMs, but also accepts all sorts of variations of archives, folders, and files. if we let archive loader handle this, it won't know where to stop, since it'd require MAME's ROM database (which contains ROM names and blob hashes) to look things up, and even then it might be confused by archive/folder structure. so assume the user provides the proper ROM directly, and handle possible errors later - : null - ); + bool allowArchives = true; + if (OpenAdvanced is OpenAdvanced_MAME) allowArchives = false; + using var file = new HawkFile(path, false, allowArchives); if (!file.Exists) return false; // if the provided file doesn't even exist, give up! CanonicalFullPath = file.CanonicalFullPath; diff --git a/src/BizHawk.Client.Common/SharpCompressDearchivalMethod.cs b/src/BizHawk.Client.Common/SharpCompressDearchivalMethod.cs index d4d6c339ee..2515d3d627 100644 --- a/src/BizHawk.Client.Common/SharpCompressDearchivalMethod.cs +++ b/src/BizHawk.Client.Common/SharpCompressDearchivalMethod.cs @@ -1,7 +1,6 @@ -#nullable enable - using System.IO; using System.Linq; +using System.Collections.Generic; using BizHawk.Common; using SharpCompress.Archives; @@ -30,6 +29,12 @@ namespace BizHawk.Client.Common public SharpCompressArchiveFile Construct(string path) => new SharpCompressArchiveFile(path); - public static readonly SharpCompressDearchivalMethod Instance = new SharpCompressDearchivalMethod(); + public static readonly SharpCompressDearchivalMethod Instance = new SharpCompressDearchivalMethod(); + + //don't try any .tar.* formats, they don't work + //don't try .gz, it's illogical (gz contains no useful archive directory information. we would need to synthesize some.) + static readonly IReadOnlyCollection archiveExts = new[] { ".zip", ".7z", ".rar" }; + + public IReadOnlyCollection AllowedArchiveExtensions { get { return archiveExts; } } } } \ No newline at end of file diff --git a/src/BizHawk.Common/HawkFile/HawkFile.cs b/src/BizHawk.Common/HawkFile/HawkFile.cs index 030461b4cb..14918ebdd8 100644 --- a/src/BizHawk.Common/HawkFile/HawkFile.cs +++ b/src/BizHawk.Common/HawkFile/HawkFile.cs @@ -71,12 +71,7 @@ namespace BizHawk.Common /// Makes a new HawkFile based on the provided path. /// Pass to only populate a few fields (those that can be computed from the string ), which is less computationally expensive. - /// - /// These file extensions are assumed to not be archives. Include the leading period in each, and use lowercase.
- /// Does not apply when is .
- /// If is passed (the default), uses which should mitigate false positives caused by weak archive detection signatures. - /// - public HawkFile([HawkFilePath] string path, bool delayIOAndDearchive = false, IReadOnlyCollection? nonArchiveExtensions = null) + public HawkFile([HawkFilePath] string path, bool delayIOAndDearchive = false, bool allowArchives = true) { if (delayIOAndDearchive) { @@ -97,23 +92,29 @@ namespace BizHawk.Common Exists = _rootExists = !string.IsNullOrEmpty(path) && new FileInfo(path).Exists; if (!_rootExists) return; - if (DearchivalMethod != null - && !(nonArchiveExtensions ?? CommonNonArchiveExtensions).Contains(Path.GetExtension(path).ToLowerInvariant()) - && DearchivalMethod.CheckSignature(path, out _, out _)) + if (DearchivalMethod != null && allowArchives) { - _extractor = DearchivalMethod.Construct(path); - try + var ext = Path.GetExtension(path).ToLowerInvariant(); + if (DearchivalMethod.AllowedArchiveExtensions.Contains(ext)) { - _archiveItems = _extractor.Scan(); - IsArchive = true; - } - catch - { - _archiveItems = null; - _extractor.Dispose(); - _extractor = null; + if (DearchivalMethod.CheckSignature(path, out _, out _)) + { + _extractor = DearchivalMethod.Construct(path); + try + { + _archiveItems = _extractor.Scan(); + IsArchive = true; + } + catch + { + _archiveItems = null; + _extractor.Dispose(); + _extractor = null; + } + } } } + if (_extractor == null) { _rootStream = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read); @@ -276,12 +277,9 @@ namespace BizHawk.Common /// Set this with an instance which can construct archive handlers as necessary for archive handling. public static IFileDearchivalMethod? DearchivalMethod; - public static readonly IReadOnlyCollection CommonNonArchiveExtensions = new[] { ".smc", ".sfc", ".dll" }; - [return: HawkFilePath] private static string MakeCanonicalName(string root, string? member) => member == null ? root : $"{root}|{member}"; - /// path / member path pair iff contains '|', otherwise private static (string, string)? SplitArchiveMemberPath([HawkFilePath] string path) { diff --git a/src/BizHawk.Common/HawkFile/IFileDearchivalMethod.cs b/src/BizHawk.Common/HawkFile/IFileDearchivalMethod.cs index ce2857b4b5..ceba27afda 100644 --- a/src/BizHawk.Common/HawkFile/IFileDearchivalMethod.cs +++ b/src/BizHawk.Common/HawkFile/IFileDearchivalMethod.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; + namespace BizHawk.Common { /// Used by to delegate archive management. @@ -6,6 +8,8 @@ namespace BizHawk.Common /// TODO could this receive a itself? possibly handy, in very clever scenarios of mounting fake files bool CheckSignature(string fileName, out int offset, out bool isExecutable); + IReadOnlyCollection AllowedArchiveExtensions { get; } + T Construct(string path); } }