fix: code audit — BSDIFF overflow, Zip traversal, ProcessExit deadlock, Trace listener, Strategy lifecycle, HubConnection, RetryPolicy (#515)

* fix: critical code audit bugs — BSDIFF overflow, Zip traversal, ProcessExit deadlock, Trace listener leak, Strategy suicide, HubConnection lifecycle, RetryPolicy status code

Comprehensive fix for ~25 bugs found in full code audit.

BSDIFF/Differential:
- Fix WriteInt64 long.MinValue overflow (BsdiffDiffer, StreamingHdiffDiffer)
- Guard (int)control[0] truncation in Dirty method (BsdiffDiffer)
- ReadFileWithBudget throws instead of silently truncating (StreamingHdiffDiffer)

Zip security:
- Add path-traversal guard in Decompress (ZipCompressionStrategy)
- Fix StartsWith ambiguity — exact match instead of substring match

Process lifecycle:
- Fix ProcessExit deadlock (Task.Run wrapper in ClientStrategy.LaunchUpgradeProcessSync)
- Move suicide from finally to success-only path (Windows/Linux/MacStrategy)
- Track appLaunched flag so Bowl failure still triggers updater exit
- GeneralTracer.Dispose removes only own listeners, not global Trace.Listeners

Configuration & lifecycle:
- Fix ProcessContract null-check ordering
- Add disposed guard + null set to UpgradeHubService (HubConnection lifecycle)
- ConfigurationMapper throws on null source instead of silent empty config
- EventManager.Reset() for re-creatable singleton

Network:
- DefaultRetryPolicy uses HttpRequestException.StatusCode (.NET 5+) or regex for status matching
- netstandard2.0 fallback uses \b(500|502|503|504)\b regex

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: add EventManager.Reset() in test constructor to isolate singleton state across test classes

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: JusterZhu

src/c#/GeneralUpdate.Core/Compress/ZipCompressionStrategy.cs

@@ -47,8 +47,12 @@ public void Compress(string sourceDirectoryName
                         var toDelArchives = new List<ZipArchiveEntry>();
                         foreach (var zipArchiveEntry in archive.Entries)
                         {
+                            // Exact match to avoid ambiguity (e.g. "foo.dll" should not match "foobar.dll").
+                            // For directory entries, also match "subdir/" prefix so nested files are replaced.
                             if (toZipedFileName != null &&
-                                (zipArchiveEntry.FullName.StartsWith(toZipedFileName) || toZipedFileName.StartsWith(zipArchiveEntry.FullName)))
+                                (zipArchiveEntry.FullName == toZipedFileName ||
+                                 zipArchiveEntry.FullName.StartsWith(toZipedFileName + "/", StringComparison.Ordinal) ||
+                                 toZipedFileName.StartsWith(zipArchiveEntry.FullName, StringComparison.Ordinal)))
                             {
                                 toDelArchives.Add(zipArchiveEntry);
                             }
@@ -83,7 +87,9 @@ public void Compress(string sourceDirectoryName
                         foreach (var zipArchiveEntry in archive.Entries)
                         {
                             if (toZipedFileName != null
-                                && (zipArchiveEntry.FullName.StartsWith(toZipedFileName) || toZipedFileName.StartsWith(zipArchiveEntry.FullName)))
+                                && (zipArchiveEntry.FullName == toZipedFileName ||
+                                    zipArchiveEntry.FullName.StartsWith(toZipedFileName + "/", StringComparison.Ordinal) ||
+                                    toZipedFileName.StartsWith(zipArchiveEntry.FullName, StringComparison.Ordinal)))
                             {
                                 toDelArchives.Add(zipArchiveEntry);
                             }
@@ -131,6 +137,8 @@ public void Decompress(string zipFilePath, string unZipDir, Encoding encoding)
                 return;
             }
 
+            var extractionRoot = Path.GetFullPath(unZipDir);
+
             using var zipToOpen = new FileStream(zipFilePath, FileMode.Open, FileAccess.ReadWrite, FileShare.Read);
             using var archive = new ZipArchive(zipToOpen, ZipArchiveMode.Read, false, encoding);
             for (int i = 0; i < archive.Entries.Count; i++)
@@ -144,20 +152,29 @@ public void Decompress(string zipFilePath, string unZipDir, Encoding encoding)
                     continue;
                 }
 
-                var filePath = directoryInfo + entryFilePath;
-                var greatFolder = Directory.GetParent(filePath);
+                // Guard against path-traversal entries (e.g. "../../evil.exe")
+                var filePath = Path.Combine(unZipDir, entryFilePath);
+                var fullTargetPath = Path.GetFullPath(filePath);
+                var rootWithSep = extractionRoot.EndsWith(dirSeparatorChar)
+                    ? extractionRoot
+                    : extractionRoot + dirSeparatorChar;
+                if (!fullTargetPath.StartsWith(rootWithSep, StringComparison.OrdinalIgnoreCase))
+                    throw new InvalidDataException(
+                        $"Zip entry path traversal detected: {entries.FullName} resolves outside extraction directory.");
+
+                var greatFolder = Directory.GetParent(fullTargetPath);
                 if (greatFolder is not null && !greatFolder.Exists)
                 {
                     greatFolder.Create();
                 }
 
-                if (File.Exists(filePath))
+                if (File.Exists(fullTargetPath))
                 {
-                    File.SetAttributes(filePath, FileAttributes.Normal);
-                    File.Delete(filePath);
+                    File.SetAttributes(fullTargetPath, FileAttributes.Normal);
+                    File.Delete(fullTargetPath);
                 }
 
-                entries.ExtractToFile(filePath);
+                entries.ExtractToFile(fullTargetPath);
             }
         }
         catch (Exception exception)

src/c#/GeneralUpdate.Core/Configuration/ConfigurationMapper.cs

@@ -72,14 +72,13 @@ public static class ConfigurationMapper
         /// </returns>
         public static UpdateContext MapToUpdateContext(UpdateRequest source, UpdateContext target = null)
         {
+            if (source == null)
+                throw new ArgumentNullException(nameof(source), "UpdateRequest source cannot be null — configuration would be empty.");
+
             // 如果 source 和 target 均未提供，则创建新实例
             if (target == null)
                 target = new UpdateContext();
 
-            // 如果 source 为 null，则直接返回空的 target
-            if (source == null)
-                return target;
-
             // 映射基类公共字段
             target.UpdateAppName = source.UpdateAppName;
             target.MainAppName = source.MainAppName;

src/c#/GeneralUpdate.Core/Configuration/ProcessContract.cs

@@ -184,8 +184,8 @@ public ProcessContract(string appName
         {
             // Validate required string parameters
             AppName = appName ?? throw new ArgumentNullException(nameof(appName));
-            if (!Directory.Exists(installPath)) throw new ArgumentException($"{nameof(installPath)} path does not exist ! {installPath}.");
             InstallPath = installPath ?? throw new ArgumentNullException(nameof(installPath));
+            if (!Directory.Exists(installPath)) throw new ArgumentException($"{nameof(installPath)} path does not exist ! {installPath}.");
             CurrentVersion = currentVersion ?? throw new ArgumentNullException(nameof(currentVersion));
             LastVersion = lastVersion ?? throw new ArgumentNullException(nameof(lastVersion));
             UpdateLogUrl = updateLogUrl;

src/c#/GeneralUpdate.Core/Differential/DefaultDirtyMatcher.cs

@@ -36,8 +36,6 @@ public class DefaultDirtyMatcher : IDirtyMatcher
         var findFile = patchFiles.FirstOrDefault(f =>
         {
             var name = Path.GetFileNameWithoutExtension(f.Name);
-            if (name.EndsWith(PatchFormat, System.StringComparison.OrdinalIgnoreCase))
-                name = name.Substring(0, name.Length - PatchFormat.Length);
             return name.Equals(oldFile.Name, System.StringComparison.OrdinalIgnoreCase);
         });
 

src/c#/GeneralUpdate.Core/Download/Policy/DefaultRetryPolicy.cs

@@ -1,6 +1,7 @@
 using System;
 using System.IO;
 using System.Net.Http;
+using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 using GeneralUpdate.Core.Download.Abstractions;
@@ -126,16 +127,25 @@ public async Task<T> ExecuteAsync<T>(Func<CancellationToken, Task<T>> action, Ca
     /// </remarks>
     private static bool IsRetryable(Exception ex)
     {
-        if (ex is OperationCanceledException) return false;
+        // TaskCanceledException derives from OperationCanceledException,
+        // so check it first — timeouts should be retryable.
         if (ex is TaskCanceledException or TimeoutException) return true;
+        if (ex is OperationCanceledException) return false;
         if (ex is IOException) return true;
         if (ex is HttpRequestException hre)
         {
+#if NET5_0_OR_GREATER
+            var statusCode = hre.StatusCode;
+            if (statusCode.HasValue)
+                return statusCode.Value == System.Net.HttpStatusCode.InternalServerError
+                    || statusCode.Value == System.Net.HttpStatusCode.BadGateway
+                    || statusCode.Value == System.Net.HttpStatusCode.ServiceUnavailable
+                    || statusCode.Value == System.Net.HttpStatusCode.GatewayTimeout;
+#endif
             var s = hre.Message ?? "";
             return s.Contains("timeout", StringComparison.OrdinalIgnoreCase)
                 || s.Contains("timed out", StringComparison.OrdinalIgnoreCase)
-                || s.Contains("500") || s.Contains("502")
-                || s.Contains("503") || s.Contains("504");
+                || Regex.IsMatch(s, @"\b(500|502|503|504)\b(?![\d./])");
         }
         return false;
     }

src/c#/GeneralUpdate.Core/Event/EventManager.cs

@@ -33,7 +33,7 @@ namespace GeneralUpdate.Core.Event
     /// </remarks>
     public class EventManager : IDisposable
     {
-        private static readonly Lazy<EventManager> _lazy = new(() => new EventManager());
+        private static volatile Lazy<EventManager> _lazy = new(() => new EventManager());
         private ConcurrentDictionary<Type, Delegate> _dicDelegates = new();
         private bool _disposed;
 
@@ -48,6 +48,15 @@ private EventManager() { }
         /// </remarks>
         public static EventManager Instance => _lazy.Value;
 
+        /// <summary>
+        /// Creates a fresh singleton instance, discarding all previously registered listeners.
+        /// Called when the update lifecycle ends and a clean slate is needed.
+        /// </summary>
+        public static void Reset()
+        {
+            _lazy = new Lazy<EventManager>(() => new EventManager());
+        }
+
         /// <summary>
         /// Registers a listener for a specified event type.
         /// </summary>

src/c#/GeneralUpdate.Core/FileSystem/StorageManager.cs

@@ -6,6 +6,7 @@
 using System.Text.Json.Serialization.Metadata;
 using System.Threading;
 using GeneralUpdate.Core.HashAlgorithms;
+using GeneralUpdate.Core;
 
 namespace GeneralUpdate.Core.FileSystem
 {
@@ -329,8 +330,9 @@ public static List<FileInfo> GetAllFiles(string path, List<string> skipDirectory
 
                 return files;
             }
-            catch
+            catch (Exception ex)
             {
+                GeneralTracer.Warn($"StorageManager.GetAllFiles failed for path '{path}': {ex.Message}");
                 return new List<FileInfo>();
             }
         }
@@ -358,8 +360,9 @@ private static List<FileInfo> GetAllfiles(string path)
 
                 return files;
             }
-            catch (Exception)
+            catch (Exception ex)
             {
+                GeneralTracer.Warn($"StorageManager.GetAllfiles failed for path '{path}': {ex.Message}");
                 return new List<FileInfo>();
             }
         }

src/c#/GeneralUpdate.Core/Hubs/UpgradeHubService.cs

@@ -18,6 +18,7 @@ public class UpgradeHubService : IUpgradeHubService
     private const string Onlineflag = "Online";
     private const string ReceiveMessageflag = "ReceiveMessage";
     private HubConnection? _connection;
+    private bool _disposed;
 
     public UpgradeHubService(string url, string? token = null, string? appkey = null)
         => _connection = BuildHubConnection(url, token, appkey);
@@ -47,24 +48,40 @@ public void AddListenerOnline(Action<string> onlineMessageCallback)
         => _connection?.On(Onlineflag, onlineMessageCallback);
 
     public void AddListenerReconnected(Func<string?, Task>? reconnectedCallback)
-        => _connection!.Reconnected += reconnectedCallback;
+    {
+        if (_disposed || _connection == null) return;
+        _connection.Reconnected += reconnectedCallback;
+    }
 
     public void AddListenerClosed(Func<Exception?, Task> closeCallback)
-        => _connection!.Closed += closeCallback;
+    {
+        if (_disposed || _connection == null) return;
+        _connection.Closed += closeCallback;
+    }
 
     public async Task StartAsync()
     {
+        if (_disposed || _connection == null)
+        {
+            GeneralTracer.Warn("UpgradeHubService.StartAsync: service is disposed or not connected.");
+            return;
+        }
         GeneralTracer.Info($"UpgradeHubService.StartAsync: connecting to SignalR hub. State={_connection?.State}");
-        await _connection!.StartAsync();
+        await _connection.StartAsync();
         GeneralTracer.Info($"UpgradeHubService.StartAsync: SignalR hub connection established. State={_connection?.State}");
     }
 
     public async Task StopAsync()
     {
+        if (_disposed || _connection == null)
+        {
+            GeneralTracer.Warn("UpgradeHubService.StopAsync: service is disposed or not connected.");
+            return;
+        }
         try
         {
             GeneralTracer.Info($"UpgradeHubService.StopAsync: stopping SignalR hub connection. State={_connection?.State}");
-            await _connection!.StopAsync();
+            await _connection.StopAsync();
             GeneralTracer.Info("UpgradeHubService.StopAsync: SignalR hub connection stopped.");
         }
         catch (Exception e)
@@ -75,11 +92,17 @@ public async Task StopAsync()
 
     public async Task DisposeAsync()
     {
+        if (_disposed) return;
+        _disposed = true;
         try
         {
-            GeneralTracer.Info("UpgradeHubService.DisposeAsync: disposing SignalR hub connection and releasing resources.");
-            await _connection!.DisposeAsync();
-            GeneralTracer.Info("UpgradeHubService.DisposeAsync: SignalR hub connection disposed.");
+            if (_connection != null)
+            {
+                GeneralTracer.Info("UpgradeHubService.DisposeAsync: disposing SignalR hub connection and releasing resources.");
+                await _connection.DisposeAsync();
+                _connection = null;
+                GeneralTracer.Info("UpgradeHubService.DisposeAsync: SignalR hub connection disposed.");
+            }
         }
         catch (Exception e)
         {

src/c#/GeneralUpdate.Core/Network/VersionService.cs

@@ -65,7 +65,7 @@ namespace GeneralUpdate.Core.Network
     public class VersionService
     {
         private static readonly HttpClient _sharedClient;
-        private static ISslValidationPolicy _globalSslPolicy = new StrictSslValidationPolicy();
+        private static volatile ISslValidationPolicy _globalSslPolicy = new StrictSslValidationPolicy();
         private static IHttpAuthProvider? _globalAuthProvider;
 
         private readonly IHttpAuthProvider _auth;

src/c#/GeneralUpdate.Core/Pipeline/DiffPipeline.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
 using GeneralUpdate.Core.Differential;
@@ -445,24 +446,42 @@ private async Task ApplyPatch(string appFilePath, string patchFilePath, Cancella
     /// deletion failures caused by read-only attributes.
     /// </para>
     /// </remarks>
-    private static void HandleDeleteList(IEnumerable<FileInfo> patchFiles, IEnumerable<FileInfo> oldFiles)
+    private static void HandleDeleteList(IEnumerable<FileInfo> patchFiles, List<FileInfo> oldFiles)
     {
         var json = patchFiles.FirstOrDefault(i => i.Name.Equals(DeleteListFileName, StringComparison.OrdinalIgnoreCase));
         if (json == null) return;
 
         var deleteFiles = StorageManager.GetJson<List<FileNode>>(json.FullName, FileNodesJsonContext.Default.ListFileNode);
         if (deleteFiles == null) return;
 
-        var hashAlgorithm = new Sha256HashAlgorithm();
-        var toDelete = oldFiles
-            .Where(old => deleteFiles.Any(del => del.Hash.SequenceEqual(hashAlgorithm.ComputeHash(old.FullName))))
+        // Build a HashSet of delete-manifest hashes (hex strings from FileNode.Hash)
+        // so each oldFile is checked in O(1) instead of O(n × m).
+        var deleteHashes = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+        foreach (var del in deleteFiles)
+        {
+            if (!string.IsNullOrEmpty(del.Hash))
+                deleteHashes.Add(del.Hash);
+        }
+        if (deleteHashes.Count == 0) return;
+
+        // Exclude .patch files from oldFiles — they are patch input, not app files.
+        var appFiles = oldFiles
+            .Where(f => !Path.GetExtension(f.FullName)
+                .Equals(PatchExtension, StringComparison.OrdinalIgnoreCase))
             .ToList();
+        if (appFiles.Count == 0) return;
 
-        foreach (var file in toDelete)
+        var hashAlgorithm = new Sha256HashAlgorithm();
+        foreach (var file in appFiles)
         {
             if (!File.Exists(file.FullName)) continue;
-            File.SetAttributes(file.FullName, FileAttributes.Normal);
-            File.Delete(file.FullName);
+
+            var fileHash = hashAlgorithm.ComputeHash(file.FullName);
+            if (fileHash != null && deleteHashes.Contains(fileHash))
+            {
+                File.SetAttributes(file.FullName, FileAttributes.Normal);
+                File.Delete(file.FullName);
+            }
         }
     }
 
@@ -501,15 +520,24 @@ private static Task CopyUnknownFiles(string appPath, string patchPath)
                 var extensionName = Path.GetExtension(file.FullName);
                 if (BlackDefaults.DefaultFormats.Contains(extensionName)) continue;
 
-                var targetFileName = file.FullName.Replace(patchPath, "").TrimStart(Path.DirectorySeparatorChar);
-                var targetPath = Path.Combine(appPath, targetFileName);
+                // Compute the relative path by stripping the patch directory prefix.
+                // Using StartsWith + Substring instead of string.Replace to avoid
+                // incorrect replacements when appPath is a substring of patchPath.
+                var patchPrefix = patchPath.TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar;
+                var comparison = RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
+                    ? StringComparison.OrdinalIgnoreCase
+                    : StringComparison.Ordinal;
+                var relativePart = file.FullName.StartsWith(patchPrefix, comparison)
+                    ? file.FullName.Substring(patchPrefix.Length)
+                    : file.FullName;
+                var targetPath = Path.Combine(appPath, relativePart);
                 var parentFolder = Directory.GetParent(targetPath);
                 if (parentFolder?.Exists == false)
                     parentFolder.Create();
 
                 // Atomic replace via temp file, same strategy as ApplyPatch.
                 // Avoids file-in-use errors when the process just exited.
-                var safeName = targetFileName.Replace(Path.DirectorySeparatorChar, '_');
+                var safeName = relativePart.Replace(Path.DirectorySeparatorChar, '_');
                 var tempPath = Path.Combine(appPath, $"{Path.GetRandomFileName()}_{safeName}");
                 File.Copy(file.FullName, tempPath, true);