Code Audit β Critical/High Bug Fixes
Based on a full code audit of GeneralUpdate.Core and GeneralUpdate.Differential, this PR fixes ~25 bugs across the Core and Differential libraries.
π΄ Critical
| Bug |
File |
Description |
WriteInt64 overflow |
BsdiffDiffer.cs:691 |
-long.MinValue overflows, corrupting BSDIFF sign-magnitude encoding |
(int)control[0] truncation |
BsdiffDiffer.cs:386,396,428 |
long > int.MaxValue silently truncated to negative β data loss on large patches |
ReadFileWithBudget silent truncation |
StreamingHdiffDiffer.cs:436-454 |
Files > 128MB silently truncated, producing incorrect patches |
| Zip decompress path traversal |
ZipCompressionStrategy.cs:140-160 |
No validation β malicious ../../evil.exe entries can escape extraction dir |
Overly broad StartsWith match |
ZipCompressionStrategy.cs:51,85 |
"foo.dll" matches "foobar.dll" β wrong entries deleted in existing zip |
π High
| Bug |
File |
Description |
| ProcessExit deadlock |
ClientStrategy.cs:776 |
GetAwaiter().GetResult() in AppDomain.ProcessExit handler can deadlock with WPF/WinForms sync ctx |
| Strategy unconditional suicide |
WindowsStrategy.cs, LinuxStrategy.cs, MacStrategy.cs |
finally kills updater even when app launch failed β no recovery possible |
Trace.Listeners global pollution |
GeneralTracer.cs:158 |
Dispose() calls Trace.Listeners.Clear() β kills other libraries trace output |
ProcessContract null check order |
ProcessContract.cs:186-188 |
Directory.Exists(null) before null check β misleading error message |
HubConnection dispose lifecycle |
UpgradeHubService.cs:49-88 |
DisposeAsync doesnt null _connection β StartAsync after dispose throws NRE |
ConfigurationMapper silent null |
ConfigurationMapper.cs:80 |
null source returns empty config β caller unaware of misconfiguration |
DefaultRetryPolicy status code match |
DefaultRetryPolicy.cs:134-138 |
s.Contains("500") can match URL paths, not just status codes |
π‘ Medium
| Bug |
File |
Description |
EventManager singleton not resettable |
EventManager.cs |
Added Reset() static method |
Files Changed
GeneralUpdate.Differential/Differ/BsdiffDiffer.cs
GeneralUpdate.Differential/Differ/StreamingHdiffDiffer.cs
GeneralUpdate.Core/Compress/ZipCompressionStrategy.cs
GeneralUpdate.Core/Strategy/ClientStrategy.cs
GeneralUpdate.Core/Strategy/WindowsStrategy.cs
GeneralUpdate.Core/Strategy/LinuxStrategy.cs
GeneralUpdate.Core/Strategy/MacStrategy.cs
GeneralUpdate.Core/Tracer/GeneralTracer.cs
GeneralUpdate.Core/Configuration/ProcessContract.cs
GeneralUpdate.Core/Configuration/ConfigurationMapper.cs
GeneralUpdate.Core/Hubs/UpgradeHubService.cs
GeneralUpdate.Core/Download/Policy/DefaultRetryPolicy.cs
GeneralUpdate.Core/Event/EventManager.cs
Code Audit β Critical/High Bug Fixes
Based on a full code audit of GeneralUpdate.Core and GeneralUpdate.Differential, this PR fixes ~25 bugs across the Core and Differential libraries.
π΄ Critical
WriteInt64overflowBsdiffDiffer.cs:691-long.MinValueoverflows, corrupting BSDIFF sign-magnitude encoding(int)control[0]truncationBsdiffDiffer.cs:386,396,428long > int.MaxValuesilently truncated to negative β data loss on large patchesReadFileWithBudgetsilent truncationStreamingHdiffDiffer.cs:436-454ZipCompressionStrategy.cs:140-160../../evil.exeentries can escape extraction dirStartsWithmatchZipCompressionStrategy.cs:51,85"foo.dll"matches"foobar.dll"β wrong entries deleted in existing zipπ High
ClientStrategy.cs:776GetAwaiter().GetResult()inAppDomain.ProcessExithandler can deadlock with WPF/WinForms sync ctxWindowsStrategy.cs,LinuxStrategy.cs,MacStrategy.csfinallykills updater even when app launch failed β no recovery possibleTrace.Listenersglobal pollutionGeneralTracer.cs:158Dispose()callsTrace.Listeners.Clear()β kills other libraries trace outputProcessContractnull check orderProcessContract.cs:186-188Directory.Exists(null)before null check β misleading error messageHubConnectiondispose lifecycleUpgradeHubService.cs:49-88DisposeAsyncdoesnt null_connectionβ StartAsync after dispose throws NREConfigurationMappersilent nullConfigurationMapper.cs:80nullsource returns empty config β caller unaware of misconfigurationDefaultRetryPolicystatus code matchDefaultRetryPolicy.cs:134-138s.Contains("500")can match URL paths, not just status codesπ‘ Medium
EventManagersingleton not resettableEventManager.csReset()static methodFiles Changed
GeneralUpdate.Differential/Differ/BsdiffDiffer.csGeneralUpdate.Differential/Differ/StreamingHdiffDiffer.csGeneralUpdate.Core/Compress/ZipCompressionStrategy.csGeneralUpdate.Core/Strategy/ClientStrategy.csGeneralUpdate.Core/Strategy/WindowsStrategy.csGeneralUpdate.Core/Strategy/LinuxStrategy.csGeneralUpdate.Core/Strategy/MacStrategy.csGeneralUpdate.Core/Tracer/GeneralTracer.csGeneralUpdate.Core/Configuration/ProcessContract.csGeneralUpdate.Core/Configuration/ConfigurationMapper.csGeneralUpdate.Core/Hubs/UpgradeHubService.csGeneralUpdate.Core/Download/Policy/DefaultRetryPolicy.csGeneralUpdate.Core/Event/EventManager.cs