-
-
Notifications
You must be signed in to change notification settings - Fork 2
fix: auto-update now performs in-place self-replace with SHA256 verification #274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -362,30 +362,102 @@ | |
| } | ||
|
|
||
| [RelayCommand] | ||
| private void InstallUpdate() | ||
| private async Task InstallUpdateAsync() | ||
| { | ||
| if (string.IsNullOrWhiteSpace(DownloadedPath) || !File.Exists(DownloadedPath)) | ||
| { | ||
| DownloadStatus = "No downloaded file to install."; | ||
| return; | ||
| } | ||
|
|
||
| if (_latest == null) | ||
| { | ||
| DownloadStatus = "No release info available."; | ||
| return; | ||
| } | ||
|
|
||
| // Step 1: Verify SHA256 hash before installing. | ||
| DownloadStatus = "Verifying file integrity..."; | ||
| var (verified, expected, actual) = await _updates.VerifyHashAsync(_latest, DownloadedPath); | ||
| if (!verified) | ||
| { | ||
| DownloadStatus = expected != null && actual != null | ||
| ? $"SHA256 mismatch — file may be corrupted. Expected: {expected[..12]}… Got: {actual[..12]}…" | ||
| : "Hash verification failed — file may be corrupted. Try downloading again."; | ||
| return; | ||
| } | ||
|
|
||
| // Step 2: Determine current executable path. | ||
| var currentExe = Environment.ProcessPath; | ||
| if (string.IsNullOrWhiteSpace(currentExe) || !File.Exists(currentExe)) | ||
| { | ||
| DownloadStatus = "Cannot determine current executable path."; | ||
| return; | ||
| } | ||
|
|
||
| // Step 3: Write an updater script that waits for this process to exit, | ||
| // copies the new exe over the old one, then launches the new version. | ||
| try | ||
| { | ||
| var pid = Environment.ProcessId; | ||
| var scriptPath = Path.Combine( | ||
| Path.GetDirectoryName(DownloadedPath)!, | ||
| "update.cmd"); | ||
Check noticeCode scanning / CodeQL Call to 'System.IO.Path.Combine' may silently drop its earlier arguments Note
Call to 'System.IO.Path.Combine' may silently drop its earlier arguments.
|
||
|
Comment on lines
+403
to
+405
|
||
|
|
||
| var script = $""" | ||
| @echo off | ||
| title SysManager Updater | ||
| echo Waiting for SysManager to close... | ||
| :wait | ||
| tasklist /FI "PID eq {pid}" 2>NUL | find /I "{pid}" >NUL | ||
| if not errorlevel 1 ( | ||
| timeout /t 1 /nobreak >NUL | ||
| goto wait | ||
| ) | ||
| echo Applying update... | ||
| copy /Y "{DownloadedPath}" "{currentExe}" >NUL | ||
| if errorlevel 1 ( | ||
| echo Update failed — could not copy file. Press any key to exit. | ||
| pause >NUL | ||
| exit /b 1 | ||
| ) | ||
|
Comment on lines
+419
to
+423
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updater copy failures become invisible because the batch runs hidden before shutdown. The error path at Line 420-Line 422 is unreachable to users in practice: the script runs with 🛠️ Minimal fix to avoid silent failures Process.Start(new ProcessStartInfo
{
FileName = "cmd.exe",
Arguments = $"/C \"{scriptPath}\"",
- UseShellExecute = false,
- CreateNoWindow = true,
- WindowStyle = ProcessWindowStyle.Hidden
+ UseShellExecute = true,
+ CreateNoWindow = false,
+ WindowStyle = ProcessWindowStyle.Normal
});Also applies to: 433-444 🤖 Prompt for AI Agents |
||
| echo Starting SysManager... | ||
| start "" "{currentExe}" | ||
| del "%~f0" | ||
| """; | ||
|
|
||
| await File.WriteAllTextAsync(scriptPath, script); | ||
|
|
||
| DownloadStatus = "Installing update — SysManager will restart..."; | ||
|
|
||
| Process.Start(new ProcessStartInfo | ||
| { | ||
| FileName = DownloadedPath, | ||
| UseShellExecute = true // lets UAC prompt if needed | ||
| FileName = "cmd.exe", | ||
| Arguments = $"/C \"{scriptPath}\"", | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true, | ||
| WindowStyle = ProcessWindowStyle.Hidden | ||
| }); | ||
| // Close the current instance so the new one takes over. | ||
|
|
||
| // Give the script a moment to start before we exit. | ||
| await Task.Delay(500); | ||
| System.Windows.Application.Current?.Shutdown(); | ||
| } | ||
| catch (IOException ex) | ||
| { | ||
| DownloadStatus = $"Update failed: {ex.Message}"; | ||
| } | ||
| catch (UnauthorizedAccessException ex) | ||
| { | ||
| DownloadStatus = $"Update failed (access denied): {ex.Message}. Try running as administrator."; | ||
| } | ||
| catch (InvalidOperationException ex) | ||
| { | ||
| DownloadStatus = $"Couldn't launch installer: {ex.Message}"; | ||
| DownloadStatus = $"Update failed: {ex.Message}"; | ||
| } | ||
| catch (System.ComponentModel.Win32Exception ex) | ||
| { | ||
| DownloadStatus = $"Couldn't launch installer: {ex.Message}"; | ||
| DownloadStatus = $"Update failed: {ex.Message}"; | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hash verification is vulnerable to TOCTOU between validation and replace.
Line 381 verifies the file, but Line 418 copies it later from disk via another process. A local file swap in-between bypasses the integrity check.
🔐 Suggested hardening (re-verify in updater script right before copy)
Also applies to: 418-419
🤖 Prompt for AI Agents