From 98e0cce8ecc08163e9fd49f2373bc0bb21ef0d50 Mon Sep 17 00:00:00 2001 From: DanS Date: Sat, 6 Jun 2026 18:35:17 -0500 Subject: [PATCH] fix(mining): harden xmrig updater per adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses confirmed findings from the multi-lens review of the updater: - Cancelable + live progress (was: download uncancelable, progress stuck at 0%, closing the dialog mid-download blocked the UI thread on the worker join). Wire a libcurl CURLOPT_XFERINFOFUNCTION that publishes byte counts and returns abort when cancel() is requested; add a Cancel button. The dialog's destructor now aborts the transfer promptly, so closing mid-download no longer freezes the UI. - Graceful "unavailable" instead of a red error on platforms with no published build (macOS / ARM): new terminal State::Unavailable rendered neutrally, not as a failure. - Install-time running guard (TOCTOU): App::isPoolMinerRunning() re-checked in the dialog before each install, so a dialog opened before mining started can't replace a live binary. - Size caps: CURLOPT_MAXFILESIZE on the download and a per-archive-member ceiling before decomphressing into memory, to bound an attacker-controlled archive. - Distinguish a local read failure of the downloaded archive from a checksum mismatch (was reported misleadingly as "possible tampering"). - Reword the dialog's verification note to "checked against the release's published SHA-256 checksum" (integrity, not authenticity — see the signing note below). Not fixed here (needs your input): WinRing0x64.sys has no per-file hash published, but it is covered by the verified archive checksum (it is inside the verified zip); and the release is not cryptographically signed — checksums and binary share one trust root. Adding a pinned-key ed25519/minisign signature is the real supply-chain hardening and needs an offline signing key + a release-process change. Both variants build; suite passes; live worker re-verified end-to-end on linux-x64. Co-Authored-By: Claude Opus 4.8 --- src/app.h | 4 +++ src/ui/windows/xmrig_download_dialog.h | 42 ++++++++++++++++++---- src/util/xmrig_updater.cpp | 50 ++++++++++++++++++++++++-- src/util/xmrig_updater.h | 8 ++++- 4 files changed, 93 insertions(+), 11 deletions(-) diff --git a/src/app.h b/src/app.h index 77f3226..696efd4 100644 --- a/src/app.h +++ b/src/app.h @@ -197,6 +197,10 @@ public: int getXmrigRequestedThreads() const { return xmrig_manager_ ? xmrig_manager_->getRequestedThreads() : 0; } + // True while the pool miner process is live — used to refuse replacing the binary under it. + bool isPoolMinerRunning() const { + return xmrig_manager_ && xmrig_manager_->isRunning(); + } // Mine-when-idle state query bool isIdleMiningActive() const { return idle_mining_active_; } diff --git a/src/ui/windows/xmrig_download_dialog.h b/src/ui/windows/xmrig_download_dialog.h index 20cddc6..07eb5c8 100644 --- a/src/ui/windows/xmrig_download_dialog.h +++ b/src/ui/windows/xmrig_download_dialog.h @@ -52,6 +52,7 @@ public: case St::Checking: renderChecking(dp, p); break; case St::UpToDate: case St::UpdateAvailable: renderPrompt(dp, p); break; + case St::Unavailable: renderUnavailable(dp, p); break; case St::Downloading: case St::Verifying: case St::Extracting: renderProgress(dp, p); break; @@ -70,11 +71,35 @@ private: static float fullW() { return ImGui::GetContentRegionAvail().x; } + // Install button that refuses (with a note) while the miner is running, re-checked live so a + // dialog opened before mining started can't replace a now-running binary (TOCTOU guard). + static void installAction(const char* label) { + using namespace material; + if (s_app->isPoolMinerRunning()) { + Type().textColored(TypeStyle::Caption, OnSurfaceMedium(), + "Stop mining before updating the miner."); + return; + } + if (TactileButton(label, ImVec2(fullW(), 0))) + s_updater->startInstall(resources::getDaemonDirectory()); + } + static void renderChecking(float, const Progress&) { using namespace material; Type().text(TypeStyle::Body2, "Checking for the latest miner…"); } + static void renderUnavailable(float, const Progress& p) { + using namespace material; + Type().text(TypeStyle::Subtitle2, "Miner updates unavailable"); + ImGui::Spacing(); + ImGui::TextWrapped("%s", p.status_text.empty() + ? "No miner build is available for this platform." + : p.status_text.c_str()); + ImGui::Spacing(); + if (TactileButton("Close", ImVec2(fullW(), 0))) s_open = false; + } + static void renderPrompt(float, const Progress& p) { using namespace material; const std::string installed = p.installed_tag.empty() ? "none" : p.installed_tag; @@ -85,17 +110,15 @@ private: ImGui::Text("Installed: %s", installed.c_str()); ImGui::Spacing(); Type().textColored(TypeStyle::Caption, OnSurfaceMedium(), - "The download is verified against its published SHA-256 checksum before install."); + "The download is checked against the release's published SHA-256 checksum before install."); ImGui::Spacing(); - if (TactileButton("Download & install", ImVec2(fullW(), 0))) - s_updater->startInstall(resources::getDaemonDirectory()); + installAction("Download & install"); } else { Type().text(TypeStyle::Subtitle2, "The miner is up to date"); ImGui::Spacing(); ImGui::Text("Installed: %s", p.latest_tag.c_str()); ImGui::Spacing(); - if (TactileButton("Reinstall", ImVec2(fullW(), 0))) - s_updater->startInstall(resources::getDaemonDirectory()); + installAction("Reinstall"); } ImGui::Spacing(); if (TactileButton("Close", ImVec2(fullW(), 0))) s_open = false; @@ -120,6 +143,12 @@ private: ImGui::Dummy(ImVec2(0, barH)); ImGui::Spacing(); ImGui::Text("%s", p.status_text.c_str()); + ImGui::Spacing(); + // Cancel aborts the in-flight transfer promptly (the curl progress callback returns abort). + if (TactileButton("Cancel", ImVec2(fullW(), 0))) { + s_updater->cancel(); + s_open = false; + } } static void renderDone(float, const Progress& p) { @@ -144,8 +173,7 @@ private: ImGui::Spacing(); ImGui::TextWrapped("%s", p.error.empty() ? "Unknown error." : p.error.c_str()); ImGui::Spacing(); - if (TactileButton("Retry", ImVec2(fullW(), 0))) - s_updater->startInstall(resources::getDaemonDirectory()); + installAction("Retry"); ImGui::Spacing(); if (TactileButton("Close", ImVec2(fullW(), 0))) s_open = false; } diff --git a/src/util/xmrig_updater.cpp b/src/util/xmrig_updater.cpp index 31d76a9..1519866 100644 --- a/src/util/xmrig_updater.cpp +++ b/src/util/xmrig_updater.cpp @@ -26,6 +26,11 @@ namespace util { namespace { +// Reject anything implausibly large for a CPU miner archive, before it can fill memory/disk — +// a defense even ahead of the checksum check (which only runs once the file is fully downloaded). +constexpr curl_off_t kMaxArchiveBytes = 64LL * 1024 * 1024; // 64 MiB +constexpr std::size_t kMaxMemberBytes = 64u * 1024 * 1024; // 64 MiB per extracted file + size_t writeStringCb(void* contents, size_t size, size_t nmemb, void* userp) { static_cast(userp)->append(static_cast(contents), size * nmemb); @@ -37,6 +42,13 @@ size_t writeFileCb(void* contents, size_t size, size_t nmemb, void* userp) return std::fwrite(contents, size, nmemb, static_cast(userp)); } +// libcurl progress callback: publish live byte counts and abort the transfer on cancel(). +int xferCb(void* clientp, curl_off_t dltotal, curl_off_t dlnow, curl_off_t, curl_off_t) +{ + auto* up = static_cast(clientp); + return up->onDownloadProgress(static_cast(dlnow), static_cast(dltotal)) ? 0 : 1; +} + std::string baseName(const std::string& path) { const auto slash = path.find_last_of("/\\"); @@ -70,10 +82,24 @@ XmrigUpdater::Progress XmrigUpdater::getProgress() const return progress_; } +bool XmrigUpdater::onDownloadProgress(double downloadedBytes, double totalBytes) +{ + { + std::lock_guard lk(mutex_); + progress_.downloaded_bytes = downloadedBytes; + if (totalBytes > 0) progress_.total_bytes = totalBytes; + progress_.percent = (progress_.total_bytes > 0) + ? static_cast(100.0 * progress_.downloaded_bytes / progress_.total_bytes) + : progress_.percent; + } + return !cancel_requested_.load(); // false -> curl aborts the transfer +} + bool XmrigUpdater::isDone() const { std::lock_guard lk(mutex_); - return progress_.state == State::Done || progress_.state == State::Failed; + return progress_.state == State::Done || progress_.state == State::Failed || + progress_.state == State::Unavailable; } void XmrigUpdater::cancel() @@ -122,6 +148,11 @@ bool XmrigUpdater::downloadToFile(const std::string& url, const std::string& des curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 30L); curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, 1024L); curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, 60L); + curl_easy_setopt(curl, CURLOPT_MAXFILESIZE_LARGE, kMaxArchiveBytes); // refuse oversized bodies + // Live progress + cancellation: the callback publishes byte counts and aborts on cancel(). + curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L); + curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, xferCb); + curl_easy_setopt(curl, CURLOPT_XFERINFODATA, this); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L); const CURLcode res = curl_easy_perform(curl); @@ -182,7 +213,7 @@ void XmrigUpdater::runCheck(std::string installedTag) progress_.installed_tag = installedTag; } if (idx < 0) { - setProgress(State::Failed, "No miner build is available for this platform (" + + setProgress(State::Unavailable, "No miner build is available for this platform (" + (token.empty() ? "unknown" : token) + ")."); return; } @@ -208,7 +239,7 @@ void XmrigUpdater::runInstall(std::string targetDir) const std::string token = currentXmrigPlatformToken(); const int idx = selectXmrigAsset(rel, token); if (idx < 0) { - setProgress(State::Failed, "No miner build is available for this platform (" + + setProgress(State::Unavailable, "No miner build is available for this platform (" + (token.empty() ? "unknown" : token) + ")."); return; } @@ -237,7 +268,17 @@ void XmrigUpdater::runInstall(std::string targetDir) setProgress(State::Verifying, "Verifying download…"); { std::ifstream f(zipPath, std::ios::binary); + if (!f) { + fs::remove(zipPath, ec); + setProgress(State::Failed, "Could not read the downloaded archive."); + return; + } const std::string bytes((std::istreambuf_iterator(f)), std::istreambuf_iterator()); + if (f.bad()) { + fs::remove(zipPath, ec); + setProgress(State::Failed, "Could not read the downloaded archive."); + return; + } const std::string actual = sha256Hex(bytes.data(), bytes.size()); const auto it = checksums.find(asset.name); if (it == checksums.end()) { @@ -274,6 +315,9 @@ void XmrigUpdater::runInstall(std::string targetDir) const std::string base = baseName(st.m_filename); if (std::find(wanted.begin(), wanted.end(), base) == wanted.end()) continue; // skip config/readme + // Reject an implausibly large member before decompressing it into memory. + if (st.m_uncomp_size > kMaxMemberBytes) { failed = true; break; } + size_t outSize = 0; void* mem = mz_zip_reader_extract_to_heap(&zip, i, &outSize, 0); if (!mem) { failed = true; break; } diff --git a/src/util/xmrig_updater.h b/src/util/xmrig_updater.h index 99d9668..aff8088 100644 --- a/src/util/xmrig_updater.h +++ b/src/util/xmrig_updater.h @@ -77,6 +77,7 @@ public: Checking, UpToDate, UpdateAvailable, + Unavailable, // no miner build is published for this platform (terminal, not an error) Downloading, Verifying, Extracting, @@ -117,7 +118,12 @@ public: void cancel(); Progress getProgress() const; - bool isDone() const; // true when state is Done or Failed (worker finished) + bool isDone() const; // true once the worker reached a terminal state (Done/Failed/Unavailable) + + // Internal: called by the libcurl progress callback. Publishes live download bytes and returns + // false to ask curl to abort (when cancel() was requested). Public only so the C callback in the + // .cpp can reach it without leaking curl types into this header. + bool onDownloadProgress(double downloadedBytes, double totalBytes); private: void runCheck(std::string installedTag);