DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 10 Wiki Activity

fix(bootstrap): harden archive names, the cancel window and quit wait

Browse Source 
The bootstrap preflight's protected names could be bypassed by names
that resolve onto them, and its cancel and quit paths had two timing
gaps.

- Entry names are judged as the file system resolves them: '\' is a
  separator, "." components are dropped and each component loses its
  trailing dots and spaces, as Win32 does ("./wallet.dat" and
  "DRAGONX.conf." name protected files). The whole archive is refused
  for an absolute or drive-rooted name, a ".." component, any ':'
  (drive letter or NTFS alternate data stream, "DRAGONX.conf::$DATA"),
  an 8.3 short name ("DRAGON~1.CON"), a component of only dots or
  spaces (other than "."), and any byte >= 0x80 or ASCII control
  character: macOS folds or ignores some Unicode (U+017F, U+212A,
  U+200C) onto a protected name, and the published bootstrap archive
  uses plain ASCII names. A refused name is echoed
  with \xNN escapes. A member is written at the path it was checked
  under, and the protected-name check runs on the resolved name.
- The user's Cancel (Settings dialog and first-run wizard) goes through
  requestCancel(). It is refused once the old chain data starts being
  replaced, and the start of that window re-checks a cancel under the
  same lock, so a click lands before the old chain is touched or not at
  all.
- Quit cancels the wizard's and the Settings bootstrap when the
  shutdown starts; the HEAD and checksum requests now end at once on
  cancel, and a cancelled HEAD keeps the partial download. A run
  cancelled before the window leaves the old chain as it was. A run
  inside it stops extracting and removes its partial chain while the
  shutdown screen shows "Cleaning up...", so it ends with no chain data
  (wallet.dat untouched; Retry reuses the zip, or the node resyncs).
  That wait is bounded: at most 2 minutes from the start of the
  shutdown, then 3 s in shutdown(), after which a worker still running
  is left to the process exit. Only a cleanup that overruns that cap
  can leave a partial chain, which dragonxd then answers with its
  reindex offer.

Tests: refusal of the Windows-resolved, non-ASCII and control-character
names with the escaped echo; the protected-name list incl. the resolved
forms and a planted -wallet file, conf and database/ that stay
byte-identical; the user-cancel window; the bounded quit wait and
safeToAbandon(); a cancel ends a HEAD request that never answers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DanS
2026-10-12 13:22:00 -05:00
co-authored by Claude Opus 5.5
parent a5185737e6
commit e7f084e3eb
7 changed files with 502 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/util/bootstrap.cpp
+139 -41
View File
@@ -32,29 +32,84 @@ static bool endsWith(const std::string& s, const std::string& suffix) {
return s.compare(s.size() - suffix.size(), suffix.size(), suffix) == 0;
}
// Reject archive member names that would escape the extraction root (zip-slip).
// The snapshot legitimately carries sub-paths (blocks/, chainstate/), so — unlike the
// updaters, which flatten to baseName() — we keep the relative path but refuse any entry
// that is absolute, drive/UNC-rooted, or contains a ".." component.
static bool isSafeArchivePath(const std::string& name) {
if (name.empty()) return false;
// The components of an archive member name as the file system resolves them, so that the zip-slip check
// and the deny-list judge the path a member really lands on. '\' is a separator, empty and "." components
// are dropped ("./wallet.dat" is wallet.dat), and each component loses its trailing dots and spaces, which
// Win32 strips ("DRAGONX.conf." opens DRAGONX.conf). Anything from a ':' on is cut off: an NTFS alternate
// data stream ("DRAGONX.conf::$DATA" is the file itself). The Windows rules apply on every platform; a
// chain snapshot carries none of these names, so treating them alike everywhere costs nothing.
// Returns false (unsafe) for an absolute or drive-rooted name, a ".." component, any ':' (drive letter,
// ADS), an 8.3 short name ("DRAGON~1.CON" can open DRAGONX.conf), a component that the stripping
// empties ("...", ".. "), and any byte >= 0x80 or ASCII control character. A chain snapshot's names are
// plain ASCII, while macOS APFS/HFS+ fold or ignore some Unicode ("databa\u017Fe" is database,
// ".coo\u212Aie" is .cookie, "DRAGONX.co\u200Cnf" is DRAGONX.conf), which no byte-wise check here could
// follow. *comps is filled either way.
static bool canonicalArchivePath(const std::string& name, std::vector<std::string>* comps) {
std::vector<std::string> out;
bool safe = !name.empty();
for (unsigned char c : name)
if (c < 0x20 || c >= 0x7f) safe = false; // control character (incl. DEL) or non-ASCII
std::string n = name;
std::replace(n.begin(), n.end(), '\\', '/'); // normalize Windows separators
if (n.front() == '/') return false; // absolute POSIX path
const char c0 = n[0];
if (n.size() >= 2 && n[1] == ':' &&
((c0 >= 'A' && c0 <= 'Z') || (c0 >= 'a' && c0 <= 'z')))
return false; // Windows drive letter (C:...)
if (!n.empty() && n.front() == '/') safe = false; // absolute POSIX path, UNC (\\server)
size_t start = 0;
while (start <= n.size()) {
for (;;) {
const size_t slash = n.find('/', start);
const std::string comp =
n.substr(start, slash == std::string::npos ? std::string::npos : slash - start);
if (comp == "..") return false; // path traversal component
std::string comp = n.substr(start, slash == std::string::npos ? std::string::npos : slash - start);
if (comp == "..") {
safe = false; // path traversal component
} else if (!comp.empty() && comp != ".") {
const size_t colon = comp.find(':');
if (colon != std::string::npos) { // drive letter (C:...) or alternate data stream
safe = false;
comp.erase(colon);
}
for (size_t t = comp.find('~'); t != std::string::npos; t = comp.find('~', t + 1))
if (t + 1 < comp.size() && comp[t + 1] >= '0' && comp[t + 1] <= '9') safe = false; // 8.3 alias
while (!comp.empty() && (comp.back() == '.' || comp.back() == ' ')) comp.pop_back();
if (comp.empty()) safe = false; // "...", ".. ", " ": no name of its own
else out.push_back(comp);
}
if (slash == std::string::npos) break;
start = slash + 1;
}
return true;
if (comps) *comps = std::move(out);
return safe;
}
// Reject archive member names that would escape the extraction root (zip-slip) or that a Windows file
// system would resolve onto another name (see canonicalArchivePath). The snapshot legitimately carries
// sub-paths (blocks/, chainstate/), so — unlike the updaters, which flatten to baseName() — we keep the
// relative path.
static bool isSafeArchivePath(const std::string& name) {
return canonicalArchivePath(name, nullptr);
}
// A member name as it may appear in an error or a log line: bytes outside printable ASCII become \xNN, so a
// refused name cannot break the log into forged lines or carry terminal escapes.
static std::string printableArchiveName(const std::string& name) {
static const char kHex[] = "0123456789ABCDEF";
std::string out;
out.reserve(name.size());
for (unsigned char c : name) {
if (c >= 0x20 && c < 0x7f) {
out.push_back(static_cast<char>(c));
} else {
out += "\\x";
out.push_back(kHex[c >> 4]);
out.push_back(kHex[c & 0xF]);
}
}
return out;
}
static std::string joinArchivePath(const std::vector<std::string>& comps) {
std::string p;
for (const std::string& c : comps) {
if (!p.empty()) p += '/';
p += c;
}
return p;
}
static std::uint64_t satAdd(std::uint64_t a, std::uint64_t b) {
@@ -76,6 +131,12 @@ static fs::space_info realSpace(const std::string& path, std::error_code& ec) {
return fs::space(path, ec);
}
// HEAD and checksum transfers: end at once on cancel. libcurl calls this about once a second even while it
// resolves or connects, so a quit never waits out their 15-30 s timeouts.
static int abortOnCancelCallback(void* clientp, curl_off_t, curl_off_t, curl_off_t, curl_off_t) {
return static_cast<const std::atomic<bool>*>(clientp)->load() ? 1 : 0;
}
static size_t writeFileCallback(void* contents, size_t size, size_t nmemb, void* userp) {
size_t total = size * nmemb;
FILE* fp = static_cast<FILE*>(userp);
@@ -274,12 +335,15 @@ void Bootstrap::installFromZip(const std::string& dataDir, const std::string& zi
}
// The destructive window starts: from here the user cannot cancel (quit still can, see cancel_requested_
// in extract), and a failure removes the partial chain again before the result is published.
// in extract), and a failure removes the partial chain again before the result is published. The flip and
// the re-check share requestCancel()'s lock: a user cancel lands before this point or is refused.
bool cancelledBeforeWindow = false;
{
std::lock_guard<std::mutex> lk(mutex_);
progress_.cancellable = false;
cancelledBeforeWindow = cancel_requested_;
}
if (cancel_requested_) {
if (cancelledBeforeWindow) {
setProgress(State::Failed, "Cancelled - the old chain data was not touched");
return;
}
@@ -313,6 +377,32 @@ void Bootstrap::cancel() {
cancel_requested_ = true;
}
bool Bootstrap::requestCancel() {
std::lock_guard<std::mutex> lk(mutex_);
if (!progress_.cancellable) return false; // the old chain data is being replaced: a no-op
cancel_requested_ = true;
return true;
}
bool Bootstrap::safeToAbandon() const {
std::lock_guard<std::mutex> lk(mutex_);
return !worker_running_ || (cancel_requested_ && progress_.cancellable);
}
bool Bootstrap::cancelAndWait(std::chrono::steady_clock::time_point deadline) {
cancel();
while (worker_running_ && std::chrono::steady_clock::now() < deadline)
std::this_thread::sleep_for(std::chrono::milliseconds(10));
if (!worker_running_) {
if (worker_.joinable()) worker_.join(); // at once: worker_running_ clears as the worker's last action
return true;
}
LOGF("[Bootstrap] Still running at quit - left to the process exit (%s)\n",
safeToAbandon() ? "the chain data was not touched"
: "the chain data may be left part-replaced; dragonxd will offer a reindex");
return false;
}
Bootstrap::Progress Bootstrap::getProgress() const {
std::lock_guard<std::mutex> lk(mutex_);
return progress_;
@@ -357,6 +447,9 @@ long long Bootstrap::getRemoteFileSize(const std::string& url) {
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10L);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L);
curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, abortOnCancelCallback);
curl_easy_setopt(curl, CURLOPT_XFERINFODATA, &cancel_requested_);
CURLcode res = curl_easy_perform(curl);
if (res != CURLE_OK) {
@@ -418,6 +511,10 @@ bool Bootstrap::download(const std::string& url, const std::string& destZip) {
// The remote size (HEAD) detects server-side changes that would corrupt a resumed download, lets a
// complete earlier download be reused, and feeds the early free-space check below.
const long long remoteSize = getRemoteFileSize(url);
if (cancel_requested_) { // the HEAD was cut short: keep a partial file for the next resume
curl_easy_cleanup(curl);
return false;
}
if (existing_bytes > 0) {
if (remoteSize > 0) {
if (existing_bytes == remoteSize) {
@@ -530,27 +627,20 @@ bool Bootstrap::download(const std::string& url, const std::string& destZip) {
// ---------------------------------------------------------------------------
bool Bootstrap::isProtectedEntry(const std::string& name) {
std::string n = name;
std::replace(n.begin(), n.end(), '\\', '/');
while (!n.empty() && n.back() == '/') n.pop_back(); // a directory entry ("database/")
n = lowerAscii(n); // Windows/macOS data dirs are case-insensitive
if (n.empty()) return false;
size_t start = 0;
for (;;) {
const size_t slash = n.find('/', start);
const std::string comp =
n.substr(start, slash == std::string::npos ? std::string::npos : slash - start);
// Judged on the name the member lands on: "./wallet.dat", "DRAGONX.conf." and "database./log.1" are
// wallet.dat, DRAGONX.conf and database/log.1 (a trailing '/' of a directory entry is an empty component).
std::vector<std::string> comps;
canonicalArchivePath(name, &comps); // an unsafe name is refused separately (isSafeArchivePath)
if (comps.empty()) return false;
for (std::string& comp : comps) {
comp = lowerAscii(comp); // Windows/macOS data dirs are case-insensitive
if (comp.rfind("wallet", 0) == 0) return true; // wallet.dat, wallet-*.dat (incl. symlinks), wallets/
if (comp == "database") return true; // the wallet's BDB environment
if (slash == std::string::npos) break;
start = slash + 1;
}
const size_t lastSlash = n.rfind('/');
const std::string base = (lastSlash == std::string::npos) ? n : n.substr(lastSlash + 1);
const std::string& base = comps.back();
if (base == ".cookie" || endsWith(base, ".conf")) return true;
// Top-level *.dat other than the node's own caches: -wallet=<name>.dat files can have any name.
if (lastSlash == std::string::npos && endsWith(base, ".dat") &&
if (comps.size() == 1 && endsWith(base, ".dat") &&
base != "fee_estimates.dat" && base != "peers.dat" && base != "banlist.dat")
return true;
return false;
@@ -574,7 +664,7 @@ Bootstrap::ArchivePlan Bootstrap::planArchive(const std::string& zipPath) {
}
const std::string filename = stat.m_filename;
if (!isSafeArchivePath(filename)) {
plan.error = "Refusing unsafe archive entry: " + filename;
plan.error = "Refusing unsafe archive entry: " + printableArchiveName(filename);
mz_zip_reader_end(&zip);
return plan;
}
@@ -638,7 +728,8 @@ bool Bootstrap::spaceSufficient(std::uint64_t needed, std::uint64_t avail, std::
// ---------------------------------------------------------------------------
bool Bootstrap::extract(const std::string& zipPath, const std::string& dataDir, std::string* err) {
// Never publishes a state: the caller removes a partial chain first and publishes last.
// Never publishes a failure or a final state (only Extracting progress): the caller removes a partial chain first
// and publishes the outcome last.
auto fail = [err](const std::string& why) {
if (err) *err = why;
return false;
@@ -666,10 +757,11 @@ bool Bootstrap::extract(const std::string& zipPath, const std::string& dataDir,
// *** SECURITY: reject zip-slip / path-traversal entries before building any path ***
// A legitimate snapshot from the project host never contains these; an entry that does
// indicates a malicious/corrupt archive, so abort rather than silently skip.
if (!isSafeArchivePath(filename)) {
DEBUG_LOGF("[Bootstrap] Unsafe archive path rejected: %s\n", filename.c_str());
std::vector<std::string> comps;
if (!canonicalArchivePath(filename, &comps)) {
DEBUG_LOGF("[Bootstrap] Unsafe archive path rejected: %s\n", printableArchiveName(filename).c_str());
mz_zip_reader_end(&zip);
return fail("Refusing to extract unsafe archive entry: " + filename);
return fail("Refusing to extract unsafe archive entry: " + printableArchiveName(filename));
}
// *** CRITICAL: Skip wallet.dat (and every other wallet*, database/, *.conf, .cookie entry) ***
@@ -677,12 +769,15 @@ bool Bootstrap::extract(const std::string& zipPath, const std::string& dataDir,
LOGF("[Bootstrap] Skipping protected archive entry: %s\n", filename.c_str());
continue;
}
// The member is written where it was checked: the canonical path ("./blocks/x" is blocks/x).
const std::string relPath = joinArchivePath(comps);
if (relPath.empty()) continue; // "./": the data dir itself
std::string destPath = dataDir;
// Ensure trailing separator
if (!destPath.empty() && destPath.back() != '/' && destPath.back() != '\\')
destPath += '/';
destPath += filename;
destPath += relPath;
if (mz_zip_reader_is_file_a_directory(&zip, i)) {
std::error_code ec;
@@ -695,7 +790,7 @@ bool Bootstrap::extract(const std::string& zipPath, const std::string& dataDir,
if (!mz_zip_reader_extract_to_file(&zip, i, destPath.c_str(), 0)) {
LOGF("[Bootstrap] Failed to extract: %s\n", filename.c_str());
// Critical chain data must extract successfully
if (filename.rfind("blocks/", 0) == 0 || filename.rfind("chainstate/", 0) == 0) {
if (relPath.rfind("blocks/", 0) == 0 || relPath.rfind("chainstate/", 0) == 0) {
mz_zip_reader_end(&zip);
return fail("Failed to extract critical file: " + filename);
}
@@ -791,6 +886,9 @@ std::string Bootstrap::downloadSmallFile(const std::string& url) {
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 15L);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L);
curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, abortOnCancelCallback);
curl_easy_setopt(curl, CURLOPT_XFERINFODATA, &cancel_requested_);
CURLcode res = curl_easy_perform(curl);
curl_easy_cleanup(curl);
src/util/bootstrap.h
+22 -3
View File
@@ -8,6 +8,7 @@
#include <vector>
#include <mutex>
#include <atomic>
#include <chrono>
#include <thread>
#include <cstdint>
#include <filesystem>
@@ -86,9 +87,24 @@ public:
void startFromZip(const std::string& dataDir, const std::string& zipPath,
DaemonUpFn daemonUp = {}, SpaceFn spaceFn = {});
/// Cancel an in-progress download/extract.
/// Cancel an in-progress download/extract. Unconditional: for quit and the destructor, which may abort
/// the destructive window too (the partial chain is then removed again).
void cancel();
/// The user's Cancel: refused (returns false, nothing changes) once the destructive window has started
/// (Progress::cancellable false). The window's start re-checks the request under the same lock, so a
/// click either lands before the old chain data is touched or not at all.
bool requestCancel();
/// Quit: true once leaving the worker to the process exit can no longer leave a part-replaced chain: it
/// has finished, or it is cancelled and has not opened the destructive window (the window's start
/// re-checks the cancel under the same lock, so it never will).
bool safeToAbandon() const;
/// Quit: cancel() and wait for the worker until `deadline`. True = finished (the object can be destroyed);
/// false = still running, and the caller must leave it to the process exit (destroying it would join).
bool cancelAndWait(std::chrono::steady_clock::time_point deadline);
/// Thread-safe read of current progress.
Progress getProgress() const;
@@ -99,7 +115,8 @@ public:
/// Format a byte count as human-readable string (e.g. "1.24 GB")
static std::string formatSize(double bytes);
/// Preflight: open the zip, refuse it if any entry is unsafe (zip-slip) or unreadable, and sum the
/// Preflight: open the zip, refuse it if any entry is unsafe (zip-slip; also any ':', an 8.3 "~N"
/// alias, or a component of only dots/spaces other than ".") or unreadable, and sum the
/// uncompressed size of the members extract() would write (directories and protected names excluded).
static ArchivePlan planArchive(const std::string& zipPath);
@@ -115,7 +132,9 @@ public:
std::uint64_t reclaimable, std::string* msg);
/// Snapshot members that are never extracted: any wallet* name, database/, *.conf, .cookie, and
/// top-level *.dat other than fee_estimates/peers/banlist.
/// top-level *.dat other than fee_estimates/peers/banlist. Judged case-insensitively on the name the
/// member lands on: "." components dropped, trailing dots/spaces and an NTFS stream suffix (":...")
/// stripped from each component, so "./wallet.dat" and "DRAGONX.conf." are protected too.
static bool isProtectedEntry(const std::string& name);
private: