perf(updater): read the downloaded archive in one bounded allocation
Before verifying a downloaded archive, both updaters slurped it into a string with an istreambuf_iterator, which grows the buffer repeatedly and reallocates as it reads a multi-megabyte file. It also placed no upper bound on the size it would read into memory. - A shared readFileExact helper sizes the file first and reads it in a single allocation, failing closed if the file is missing, empty, larger than a caller-supplied cap, or comes up short. A truncated read can never reach the SHA-256 / signature verifier. - Both updaters use it with the existing max-archive cap. Tests cover a good read, a read exactly at the cap, over the cap, an empty file, and a missing file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -281,14 +281,8 @@ void DaemonUpdater::installResolved(const std::string& targetDir, const DaemonRe
|
||||
setProgress(State::Verifying, "Verifying download…");
|
||||
std::string bytes; // kept in scope through extraction so we extract the VERIFIED buffer (I-01)
|
||||
{
|
||||
std::ifstream f(zipPath, std::ios::binary);
|
||||
if (!f) {
|
||||
fs::remove(zipPath, ec);
|
||||
setProgress(State::Failed, "Could not read the downloaded archive.");
|
||||
return;
|
||||
}
|
||||
bytes.assign(std::istreambuf_iterator<char>(f), std::istreambuf_iterator<char>());
|
||||
if (f.bad()) {
|
||||
if (!readFileExact(zipPath, bytes,
|
||||
static_cast<std::uintmax_t>(kMaxArchiveBytes))) {
|
||||
fs::remove(zipPath, ec);
|
||||
setProgress(State::Failed, "Could not read the downloaded archive.");
|
||||
return;
|
||||
|
||||
@@ -8,8 +8,10 @@
|
||||
|
||||
#include <curl/curl.h>
|
||||
|
||||
#include <cstdint>
|
||||
#include <cstdio>
|
||||
#include <filesystem>
|
||||
#include <fstream>
|
||||
#include <system_error>
|
||||
#include <utility>
|
||||
|
||||
@@ -155,5 +157,18 @@ bool httpDownloadToFile(const std::string& url, const std::string& destPath,
|
||||
return true;
|
||||
}
|
||||
|
||||
bool readFileExact(const std::string& path, std::string& out, std::uintmax_t maxBytes)
|
||||
{
|
||||
out.clear();
|
||||
std::ifstream f(path, std::ios::binary | std::ios::ate);
|
||||
if (!f) return false;
|
||||
const std::streamoff size = f.tellg();
|
||||
if (size <= 0 || static_cast<std::uintmax_t>(size) > maxBytes) return false;
|
||||
f.seekg(0, std::ios::beg);
|
||||
out.resize(static_cast<std::size_t>(size));
|
||||
if (!f.read(out.data(), size) || f.gcount() != size) { out.clear(); return false; }
|
||||
return true;
|
||||
}
|
||||
|
||||
} // namespace util
|
||||
} // namespace dragonx
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
|
||||
#include <atomic>
|
||||
#include <cstddef>
|
||||
#include <cstdint>
|
||||
#include <functional>
|
||||
#include <string>
|
||||
|
||||
@@ -63,5 +64,10 @@ bool httpDownloadToFile(const std::string& url, const std::string& destPath,
|
||||
const char* logTag, long long maxBytes,
|
||||
const std::function<bool(double downloaded, double total)>& onProgress);
|
||||
|
||||
// Read the whole file at `path` into `out` in one allocation. Fails closed (returns false, `out`
|
||||
// cleared) if the file is missing, empty, larger than `maxBytes`, or a short read occurs. Used by the
|
||||
// updaters before SHA-256 / signature verification, so a truncated read can never reach the verifier.
|
||||
bool readFileExact(const std::string& path, std::string& out, std::uintmax_t maxBytes);
|
||||
|
||||
} // namespace util
|
||||
} // namespace dragonx
|
||||
|
||||
@@ -269,14 +269,8 @@ void XmrigUpdater::installResolved(const std::string& targetDir, const XmrigRele
|
||||
setProgress(State::Verifying, "Verifying download…");
|
||||
std::string bytes; // kept in scope through extraction so we extract the VERIFIED buffer (I-01)
|
||||
{
|
||||
std::ifstream f(zipPath, std::ios::binary);
|
||||
if (!f) {
|
||||
fs::remove(zipPath, ec);
|
||||
setProgress(State::Failed, "Could not read the downloaded archive.");
|
||||
return;
|
||||
}
|
||||
bytes.assign(std::istreambuf_iterator<char>(f), std::istreambuf_iterator<char>());
|
||||
if (f.bad()) {
|
||||
if (!readFileExact(zipPath, bytes,
|
||||
static_cast<std::uintmax_t>(kMaxArchiveBytes))) {
|
||||
fs::remove(zipPath, ec);
|
||||
setProgress(State::Failed, "Could not read the downloaded archive.");
|
||||
return;
|
||||
|
||||
@@ -18413,12 +18413,45 @@ void testMigrationWantsBalanceScan()
|
||||
EXPECT_FALSE(migrationWantsBalanceScan(false, true, 1, -1.0)); // syncing defers the automatic scan
|
||||
}
|
||||
|
||||
void testReadFileExact()
|
||||
{
|
||||
namespace fs = std::filesystem;
|
||||
const fs::path dir = fs::temp_directory_path() / "dragonx_readfileexact_test";
|
||||
std::error_code ec;
|
||||
fs::create_directories(dir, ec);
|
||||
const std::string good = (dir / "good.bin").string();
|
||||
const std::string empty = (dir / "empty.bin").string();
|
||||
std::string payload;
|
||||
for (int i = 0; i < 1000; ++i) payload.push_back(static_cast<char>(i % 251));
|
||||
payload.push_back('\0');
|
||||
{ std::ofstream o(good, std::ios::binary); o.write(payload.data(), static_cast<std::streamsize>(payload.size())); }
|
||||
{ std::ofstream o(empty, std::ios::binary); }
|
||||
|
||||
std::string out = "stale";
|
||||
EXPECT_TRUE(dragonx::util::readFileExact(good, out, 1 << 20));
|
||||
EXPECT_TRUE(out == payload);
|
||||
EXPECT_TRUE(dragonx::util::readFileExact(good, out, payload.size())); // exactly at the cap
|
||||
EXPECT_TRUE(out == payload);
|
||||
|
||||
out = "stale";
|
||||
EXPECT_FALSE(dragonx::util::readFileExact(good, out, payload.size() - 1)); // over the cap
|
||||
EXPECT_TRUE(out.empty());
|
||||
out = "stale";
|
||||
EXPECT_FALSE(dragonx::util::readFileExact(empty, out, 1 << 20));
|
||||
EXPECT_TRUE(out.empty());
|
||||
out = "stale";
|
||||
EXPECT_FALSE(dragonx::util::readFileExact((dir / "missing.bin").string(), out, 1 << 20));
|
||||
EXPECT_TRUE(out.empty());
|
||||
fs::remove_all(dir, ec);
|
||||
}
|
||||
|
||||
int main()
|
||||
{
|
||||
testConnectionConfig();
|
||||
testClassifyRpcFailure();
|
||||
testClassifyUnlockFailure();
|
||||
testHttpSinkAccept();
|
||||
testReadFileExact();
|
||||
testSecureWipeAndRemove();
|
||||
testFormatHttpError();
|
||||
testRpcFailureKindOf();
|
||||
|
||||
Reference in New Issue
Block a user