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

security(exposure): owner-only ACLs on Windows, auto-clearing export copy, require SHA-256 for bootstrap

Browse Source 
- writeFileAtomically honored restrictPermissions only on POSIX (0600); on
  Windows it was a no-op, so exported keys / seed backups / the vault / chat
  exports inherited the parent dir's ACL. It now builds a PROTECTED owner-only
  DACL (current-user SID, SE_DACL_PROTECTED) on the temp file before the atomic
  rename; best-effort, falling back to default ACLs on failure. (Verified it
  compiles under mingw-w64; the POSIX path is unchanged.)
- The key-export dialog's Copy wrote the private key to the plain, permanent
  clipboard; it now goes through copySecretToClipboard() (45s armed auto-wipe +
  its own "copied, auto-clears" notice), like the seed backup copy.
- Bootstrap archive verification dropped the collision-prone MD5 fallback: it now
  requires a published SHA-256 and fails closed otherwise. (Authenticity vs a
  malicious server still needs a detached ed25519 signature like the updaters —
  a separate release-coordinated change.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
DanS
2026-10-19 10:34:47 -05:00
co-authored by Claude Opus 4.8
parent 6b0f2eda22
commit 31e9c2ee2c
3 changed files with 71 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/app.cpp
+3 -2
View File
@@ -4344,8 +4344,9 @@ void App::renderExportKeyDialog()
ImGui::InputTextMultiline("##exportresult", export_result_, sizeof(export_result_),
ImVec2(-1, 60.0f * dp), ImGuiInputTextFlags_ReadOnly);
if (m::TactileButton(TR("copy"))) {
ImGui::SetClipboardText(export_result_);
ui::Notifications::instance().info(TR("copied_to_clipboard"));
// A private key is a secret — route through the auto-clearing clipboard path (45s armed wipe
// + its own "copied, auto-clears" toast) instead of a plain, permanent clipboard write.
copySecretToClipboard(export_result_);
}
} else if (export_error_) {
ImGui::Spacing();
src/util/bootstrap.cpp
+22 -45
View File
@@ -1125,56 +1125,33 @@ bool Bootstrap::verifyChecksums(const std::string& zipPath, const std::string& b
std::string expectedSha = parseChecksumFile(sha256Content, 64);
const bool haveSHA256 = !expectedSha.empty();
std::string expectedMd5;
bool haveMD5 = false;
if (!haveSHA256) { // MD5 only as a fallback when no SHA-256 is published
std::string md5Content = httpGetString(baseUrl + "/" + kZipName + ".md5", "[bootstrap]", opt);
if (cancel_requested_) return false;
expectedMd5 = parseChecksumFile(md5Content, 32);
haveMD5 = !expectedMd5.empty();
}
if (!haveSHA256 && !haveMD5) {
DEBUG_LOGF("[Bootstrap] No checksum published - refusing to install an unverified archive\n");
setProgress(State::Failed, "Verification failed: no checksum was published for the bootstrap. "
// SHA-256 is now required: the collision-prone MD5 fallback was removed. (A same-origin checksum
// only protects against transport corruption, not a malicious server — true authenticity needs a
// detached ed25519 signature like the daemon/xmrig updaters; that is a separate release-coordinated
// change, since it requires publishing per-archive .sig files and pinning a key.)
if (!haveSHA256) {
DEBUG_LOGF("[Bootstrap] No SHA-256 published - refusing to install an unverified archive\n");
setProgress(State::Failed, "Verification failed: no SHA-256 checksum was published for the bootstrap. "
"Refusing to install an unverified archive.");
return false; // fail CLOSED; archive is KEPT
}
if (haveSHA256) {
setProgress(State::Verifying, "Verifying SHA-256...", 0, 1);
std::string actual = computeSHA256(zipPath, 0.0f, 100.0f);
if (cancel_requested_) return false;
if (actual.empty()) { setProgress(State::Failed, "SHA-256 verification error"); return false; }
if (expectedSha != actual) {
last_verify_mismatch_ = true;
DEBUG_LOGF("[Bootstrap] SHA-256 MISMATCH!\n expected: %s\n actual: %s\n",
expectedSha.c_str(), actual.c_str());
setProgress(State::Failed,
"SHA-256 mismatch \xe2\x80\x94 the server's checksum file may be out of date.\n"
"Expected: " + expectedSha.substr(0, 16) + "...\n"
"Got: " + actual.substr(0, 16) + "...\n"
"Try again or use a fresh download.");
return false;
}
DEBUG_LOGF("[Bootstrap] SHA-256 verified: %s\n", actual.c_str());
} else { // haveMD5
setProgress(State::Verifying, "Verifying MD5...", 0, 1);
std::string actual = computeMD5(zipPath, 0.0f, 100.0f);
if (cancel_requested_) return false;
if (actual.empty()) { setProgress(State::Failed, "MD5 verification error"); return false; }
if (expectedMd5 != actual) {
last_verify_mismatch_ = true;
DEBUG_LOGF("[Bootstrap] MD5 MISMATCH!\n expected: %s\n actual: %s\n",
expectedMd5.c_str(), actual.c_str());
setProgress(State::Failed,
"MD5 mismatch \xe2\x80\x94 the server's checksum file may be out of date.\n"
"Expected: " + expectedMd5 + "\nGot: " + actual + "\n"
"Try again or use a fresh download.");
return false;
}
DEBUG_LOGF("[Bootstrap] MD5 verified: %s\n", actual.c_str());
setProgress(State::Verifying, "Verifying SHA-256...", 0, 1);
std::string actual = computeSHA256(zipPath, 0.0f, 100.0f);
if (cancel_requested_) return false;
if (actual.empty()) { setProgress(State::Failed, "SHA-256 verification error"); return false; }
if (expectedSha != actual) {
last_verify_mismatch_ = true;
DEBUG_LOGF("[Bootstrap] SHA-256 MISMATCH!\n expected: %s\n actual: %s\n",
expectedSha.c_str(), actual.c_str());
setProgress(State::Failed,
"SHA-256 mismatch \xe2\x80\x94 the server's checksum file may be out of date.\n"
"Expected: " + expectedSha.substr(0, 16) + "...\n"
"Got: " + actual.substr(0, 16) + "...\n"
"Try again or use a fresh download.");
return false;
}
DEBUG_LOGF("[Bootstrap] SHA-256 verified: %s\n", actual.c_str());
setProgress(State::Verifying, "Checksums verified \xe2\x9c\x93", 100.0, 100.0);
return true;
src/util/platform.cpp
+46 -3
View File
@@ -25,6 +25,7 @@
#include <shlobj.h>
#include <psapi.h>
#include <tlhelp32.h>
#include <aclapi.h> // SetEntriesInAcl / EXPLICIT_ACCESS — owner-only DACL for restricted writes
#else
#include <unistd.h>
#include <fcntl.h>
@@ -361,15 +362,54 @@ bool Platform::writeFileAtomically(const std::string& path,
const std::string tmp = path + ".tmp";
#ifdef _WIN32
// When restrictPermissions, build a PROTECTED owner-only DACL so the secret file (exported keys,
// seed backup, vault, chat export) is readable only by the current user — not whatever the parent
// dir's inherited ACL grants. Best-effort: on any failure fall back to default ACLs (the per-user
// profile dir still protects it). The ACL is set on the temp file; MoveFileEx is a same-volume
// rename, so the explicit (non-inherited) DACL carries to the final path.
PACL dacl = nullptr;
PTOKEN_USER tokenUser = nullptr;
SECURITY_DESCRIPTOR sd;
SECURITY_ATTRIBUTES saOwner{};
SECURITY_ATTRIBUTES* psa = nullptr;
if (restrictPermissions) {
HANDLE tok = nullptr;
if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &tok)) {
DWORD need = 0;
GetTokenInformation(tok, TokenUser, nullptr, 0, &need);
if (need > 0) {
tokenUser = static_cast<PTOKEN_USER>(LocalAlloc(LPTR, need));
if (tokenUser && GetTokenInformation(tok, TokenUser, tokenUser, need, &need)) {
EXPLICIT_ACCESSA ea{};
ea.grfAccessPermissions = GENERIC_ALL;
ea.grfAccessMode = SET_ACCESS;
ea.grfInheritance = NO_INHERITANCE;
ea.Trustee.TrusteeForm = TRUSTEE_IS_SID;
ea.Trustee.TrusteeType = TRUSTEE_IS_USER;
ea.Trustee.ptstrName = static_cast<LPSTR>(tokenUser->User.Sid);
if (SetEntriesInAclA(1, &ea, nullptr, &dacl) == ERROR_SUCCESS &&
InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION) &&
SetSecurityDescriptorControl(&sd, SE_DACL_PROTECTED, SE_DACL_PROTECTED) &&
SetSecurityDescriptorDacl(&sd, TRUE, dacl, FALSE)) {
saOwner.nLength = sizeof(saOwner);
saOwner.lpSecurityDescriptor = &sd;
saOwner.bInheritHandle = FALSE;
psa = &saOwner;
}
}
}
CloseHandle(tok);
}
}
{
// One handle for write + flush. CreateFileA with no SECURITY_ATTRIBUTES is NOT inheritable: the old
// std::ofstream (CRT) handle was, so a child spawned with bInheritHandles=TRUE at that moment (xmrig,
// runHiddenCapture) kept "<path>.tmp" open without FILE_SHARE_DELETE and every later MoveFileEx
// failed while it ran. Also no second share-mode-0 open, which silently skipped the flush whenever
// AV / the indexer had the new file open.
HANDLE h = CreateFileA(tmp.c_str(), GENERIC_WRITE, 0, nullptr,
HANDLE h = CreateFileA(tmp.c_str(), GENERIC_WRITE, 0, psa,
CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
if (h == INVALID_HANDLE_VALUE) return false;
if (h == INVALID_HANDLE_VALUE) { if (dacl) LocalFree(dacl); if (tokenUser) LocalFree(tokenUser); return false; }
bool ok = true;
size_t written = 0;
while (written < content.size()) {
@@ -382,9 +422,12 @@ bool Platform::writeFileAtomically(const std::string& path,
// Flush the temp file's contents to stable storage before the rename.
if (ok && !FlushFileBuffers(h)) ok = false;
CloseHandle(h);
// The DACL is baked into the temp file's security descriptor at CreateFileA time, so release the
// builders now; every path below (including the early returns) is past this point.
if (dacl) LocalFree(dacl);
if (tokenUser) LocalFree(tokenUser);
if (!ok) { DeleteFileA(tmp.c_str()); return false; }
}
(void)restrictPermissions; // owner-only ACLs rely on the per-user profile dir on Windows
// MoveFileEx with REPLACE_EXISTING is atomic on NTFS; WRITE_THROUGH makes it durable. AV / the search
// indexer can hold the destination briefly (ACCESS_DENIED / SHARING_VIOLATION / LOCK_VIOLATION): retry
// a few times (20+40+80+160 ms worst case, only on a write that used to just fail) instead of dropping it.