From ee6cac41c491f2e0da9facf3745df052a77205f7 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 10 Jun 2026 14:05:43 -0500 Subject: [PATCH] fix(robustness): guard malformed RPC error JSON + send single-flight (audit #7-8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - rpc_client::callRaw: a daemon error object is no longer assumed to carry a string "message" — a malformed error now yields a clean "RPC error: " instead of throwing a json type-exception from .get(). - sendTransaction (full-node): add a single-flight guard so a rapid double-click can't issue two z_sendmany before the first returns its opid. The lite path already guarded this; the send form guards it in the UI, but the controller entry point now does too. (#9 from the audit was mostly false positives on verification — all popen sites already null-check and the xmrig download FILE* path has no throwing calls. The payment-URI checksum idea was dropped: the send flow already checksum-validates the recipient before broadcasting, and tightening the parser would reject the placeholder addresses the existing test relies on; added a comment noting this is format-only by design.) Co-Authored-By: Claude Opus 4.8 --- src/app_network.cpp | 11 ++++++++++- src/rpc/rpc_client.cpp | 9 ++++++++- src/util/payment_uri.cpp | 12 +++++++----- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/app_network.cpp b/src/app_network.cpp index 0468459..60fecee 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -2240,7 +2240,16 @@ void App::sendTransaction(const std::string& from, const std::string& to, if (callback) callback(false, "Not connected"); return; } - + + // Single-flight guard: a rapid double-click (or any second caller) must not issue two + // z_sendmany calls before the first returns its opid. The send form already guards this in the + // UI, but the controller entry point must not depend on that. (send_submissions_in_flight_ is + // main-thread only: ++ here, -- in the worker's main-thread result callback.) + if (send_submissions_in_flight_ > 0) { + if (callback) callback(false, "A transaction is already being submitted — please wait."); + return; + } + // Check that we have the spending key for the from address if (!from.empty() && from[0] == 'z') { bool spendable = false; diff --git a/src/rpc/rpc_client.cpp b/src/rpc/rpc_client.cpp index 58d47f1..2cb5a62 100644 --- a/src/rpc/rpc_client.cpp +++ b/src/rpc/rpc_client.cpp @@ -470,7 +470,14 @@ std::string RPCClient::callRaw(const std::string& method, const json& params) // Parse with ordered_json to preserve the daemon's original key order nlohmann::ordered_json oj = nlohmann::ordered_json::parse(response_data); if (oj.contains("error") && !oj["error"].is_null()) { - std::string err_msg = oj["error"]["message"].get(); + // A daemon error object normally has a string "message", but don't assume it — a malformed + // error (missing/non-string message) must yield a clean RPC error, not a json type-exception. + const auto& err = oj["error"]; + std::string err_msg; + if (err.is_object() && err.contains("message") && err["message"].is_string()) + err_msg = err["message"].get(); + else + err_msg = err.dump(); throw std::runtime_error("RPC error: " + err_msg); } diff --git a/src/util/payment_uri.cpp b/src/util/payment_uri.cpp index e7ed5d6..bdcfd81 100644 --- a/src/util/payment_uri.cpp +++ b/src/util/payment_uri.cpp @@ -115,19 +115,21 @@ PaymentURI parsePaymentURI(const std::string& uri) return result; } - // Basic address format check + // Basic address format check. NOTE: this is format-only by design — the send flow + // checksum-validates the recipient (isValidBase58Check / shielded check) before broadcasting, + // so an invalid-checksum address parsed here can never actually be sent to. bool validFormat = false; - + // z-address: starts with 'zs' and is 78+ chars if (result.address[0] == 'z' && result.address.size() >= 78) { validFormat = true; } - // t-address: starts with 'R' (DragonX) or 't' (HUSH) and is ~34 chars - else if ((result.address[0] == 'R' || result.address[0] == 't') && + // t-address: starts with 'R' (DragonX) or 't' (HUSH) and is ~34 chars + else if ((result.address[0] == 'R' || result.address[0] == 't') && result.address.size() >= 26 && result.address.size() <= 36) { validFormat = true; } - + if (!validFormat) { result.error = "Invalid address format"; return result;