fix(rpc,ui): close the last secret residues found by the final-gate review
Two verified medium leaks in the B7 scrub chain: - parseRpcResult parses the body into a local `response` tree and returns a COPY of response["result"] (operator[] yields an lvalue ref, so the by-value return copy-constructs). The local tree — holding its own heap copy of the secret — was then freed without zeroing, so callSecret/callSecretString still left one un-scrubbed copy. Add scrubJsonSecrets() (recursive string zero) and a scrubSource flag; the secret paths opt in, wiping the tree before it frees. The secret export chain is now fully covered: raw body → parse tree → result copy → caller-owned string. - key_export_dialog cleared s_key with plain std::string::clear() on the Close button, the scrim/Esc dismiss path, and the QR cache (s_qr_cached) — leaving the displayed private/spending key in freed heap on the ordinary close paths. Route all three through wallet::secureWipeLiteSecret (zero-then-clear), matching show()/hide(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -23,6 +23,17 @@ namespace rpc {
|
|||||||
|
|
||||||
namespace {
|
namespace {
|
||||||
|
|
||||||
|
// Recursively zero every string value in a JSON tree in place — used to wipe a discarded parse tree
|
||||||
|
// that held a secret (B7). Operates on the underlying std::string buffers via get_ref.
|
||||||
|
void scrubJsonSecrets(nlohmann::json& j) {
|
||||||
|
if (j.is_string()) {
|
||||||
|
auto& s = j.get_ref<std::string&>();
|
||||||
|
if (!s.empty()) sodium_memzero(&s[0], s.size());
|
||||||
|
} else if (j.is_object() || j.is_array()) {
|
||||||
|
for (auto& el : j) scrubJsonSecrets(el);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
std::mutex g_trace_mutex;
|
std::mutex g_trace_mutex;
|
||||||
RPCClient::TraceCallback g_trace_callback;
|
RPCClient::TraceCallback g_trace_callback;
|
||||||
std::atomic_bool g_trace_enabled{false};
|
std::atomic_bool g_trace_enabled{false};
|
||||||
@@ -317,7 +328,7 @@ std::string RPCClient::performCall(const std::string& method, const json& params
|
|||||||
return response_data;
|
return response_data;
|
||||||
}
|
}
|
||||||
|
|
||||||
json RPCClient::parseRpcResult(long httpCode, const std::string& body)
|
json RPCClient::parseRpcResult(long httpCode, const std::string& body, bool scrubSource)
|
||||||
{
|
{
|
||||||
// Bitcoin/Hush RPC returns HTTP 500 for application-level errors
|
// Bitcoin/Hush RPC returns HTTP 500 for application-level errors
|
||||||
// (insufficient funds, bad params, etc.) with a valid JSON body.
|
// (insufficient funds, bad params, etc.) with a valid JSON body.
|
||||||
@@ -355,7 +366,9 @@ json RPCClient::parseRpcResult(long httpCode, const std::string& body)
|
|||||||
throw RpcError(errCode, "RPC error: " + err_msg);
|
throw RpcError(errCode, "RPC error: " + err_msg);
|
||||||
}
|
}
|
||||||
|
|
||||||
return response["result"];
|
json result = response["result"]; // a COPY of the subobject (operator[] yields an lvalue ref)
|
||||||
|
if (scrubSource) scrubJsonSecrets(response); // zero the discarded tree's secret before it frees (B7)
|
||||||
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
json RPCClient::call(const std::string& method, const json& params)
|
json RPCClient::call(const std::string& method, const json& params)
|
||||||
@@ -382,7 +395,7 @@ json RPCClient::callSecret(const std::string& method, const json& params)
|
|||||||
long http_code = 0;
|
long http_code = 0;
|
||||||
std::string response_data = performCall(method, params, http_code);
|
std::string response_data = performCall(method, params, http_code);
|
||||||
try {
|
try {
|
||||||
json result = parseRpcResult(http_code, response_data);
|
json result = parseRpcResult(http_code, response_data, /*scrubSource=*/true);
|
||||||
if (!response_data.empty()) sodium_memzero(&response_data[0], response_data.size());
|
if (!response_data.empty()) sodium_memzero(&response_data[0], response_data.size());
|
||||||
return result;
|
return result;
|
||||||
} catch (...) {
|
} catch (...) {
|
||||||
@@ -401,7 +414,7 @@ std::string RPCClient::callSecretString(const std::string& method, const json& p
|
|||||||
long http_code = 0;
|
long http_code = 0;
|
||||||
std::string response_data = performCall(method, params, http_code);
|
std::string response_data = performCall(method, params, http_code);
|
||||||
try {
|
try {
|
||||||
json result = parseRpcResult(http_code, response_data);
|
json result = parseRpcResult(http_code, response_data, /*scrubSource=*/true);
|
||||||
std::string out;
|
std::string out;
|
||||||
if (result.is_string()) {
|
if (result.is_string()) {
|
||||||
// Copy the secret out, then zero the json node's OWN heap buffer — parseRpcResult's
|
// Copy the secret out, then zero the json node's OWN heap buffer — parseRpcResult's
|
||||||
|
|||||||
@@ -270,8 +270,10 @@ private:
|
|||||||
// hold curl_mutex_ and have verified impl_->curl.
|
// hold curl_mutex_ and have verified impl_->curl.
|
||||||
std::string performCall(const std::string& method, const json& params, long& httpCodeOut);
|
std::string performCall(const std::string& method, const json& params, long& httpCodeOut);
|
||||||
// Centralizes the HTTP-code check and JSON error->RpcError extraction, returning
|
// Centralizes the HTTP-code check and JSON error->RpcError extraction, returning
|
||||||
// response["result"] on success.
|
// response["result"] on success. When scrubSource is true (secret-bearing calls), the intermediate
|
||||||
static json parseRpcResult(long httpCode, const std::string& body);
|
// parse tree's string values are zeroed before it is discarded — parseRpcResult returns a COPY of
|
||||||
|
// the result node, so its own tree would otherwise free the secret un-wiped (B7).
|
||||||
|
static json parseRpcResult(long httpCode, const std::string& body, bool scrubSource = false);
|
||||||
|
|
||||||
// Splits a UnifiedCallback into the (Callback, ErrorCallback) pair used by doRPC.
|
// Splits a UnifiedCallback into the (Callback, ErrorCallback) pair used by doRPC.
|
||||||
static std::pair<Callback, ErrorCallback> splitUnified(UnifiedCallback cb);
|
static std::pair<Callback, ErrorCallback> splitUnified(UnifiedCallback cb);
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ std::string KeyExportDialog::s_error;
|
|||||||
void KeyExportDialog::releaseQr()
|
void KeyExportDialog::releaseQr()
|
||||||
{
|
{
|
||||||
if (s_qr_tex) { FreeQRTexture(s_qr_tex); s_qr_tex = 0; }
|
if (s_qr_tex) { FreeQRTexture(s_qr_tex); s_qr_tex = 0; }
|
||||||
s_qr_cached.clear();
|
wallet::secureWipeLiteSecret(s_qr_cached); // held a plaintext copy of the key for the QR
|
||||||
s_show_qr = false;
|
s_show_qr = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -318,8 +318,8 @@ void KeyExportDialog::render(App* app)
|
|||||||
|
|
||||||
if (material::TactileButton(TR("close"), ImVec2(button_width, 0), S.resolveFont(closeBtn.font))) {
|
if (material::TactileButton(TR("close"), ImVec2(button_width, 0), S.resolveFont(closeBtn.font))) {
|
||||||
s_open = false;
|
s_open = false;
|
||||||
// Clear sensitive data
|
// Zero the secret, don't just drop the buffer (leaves the key in freed heap otherwise).
|
||||||
s_key.clear();
|
wallet::secureWipeLiteSecret(s_key);
|
||||||
s_show_key = false;
|
s_show_key = false;
|
||||||
releaseQr();
|
releaseQr();
|
||||||
}
|
}
|
||||||
@@ -327,9 +327,9 @@ void KeyExportDialog::render(App* app)
|
|||||||
material::EndOverlayDialog();
|
material::EndOverlayDialog();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Dialog dismissed any other way (scrim click / Esc): drop the key + its QR texture.
|
// Dialog dismissed any other way (scrim click / Esc): wipe the key + its QR texture.
|
||||||
if (!s_open) {
|
if (!s_open) {
|
||||||
if (!s_key.empty()) s_key.clear();
|
wallet::secureWipeLiteSecret(s_key);
|
||||||
s_show_key = false;
|
s_show_key = false;
|
||||||
releaseQr();
|
releaseQr();
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user