fix(mining): harden xmrig updater per adversarial review
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<std::string*>(userp)->append(static_cast<char*>(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<FILE*>(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<XmrigUpdater*>(clientp);
|
||||
return up->onDownloadProgress(static_cast<double>(dlnow), static_cast<double>(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<std::mutex> lk(mutex_);
|
||||
progress_.downloaded_bytes = downloadedBytes;
|
||||
if (totalBytes > 0) progress_.total_bytes = totalBytes;
|
||||
progress_.percent = (progress_.total_bytes > 0)
|
||||
? static_cast<float>(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<std::mutex> 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<char>(f)), std::istreambuf_iterator<char>());
|
||||
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; }
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user