From 13788b5dd05a5d31f14126f3a926c854cc9946d1 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 22 Jul 2026 01:56:13 -0500 Subject: [PATCH] fix(security): harden untrusted-input parsing (Tier 4 audit items) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - export: neutralize CSV/spreadsheet formula injection in transaction export — escapeCSV() now prefixes fields beginning with = + - @ / tab / CR with a single quote so a hostile memo/address cannot execute on open (F13-1). - console: preserve explicitly-quoted empty arguments in ParseConsoleCommandArgs so a "" no longer collapses and shifts later positional RPC params; adds a regression assertion to testConsoleInputModel (F14-1). - send: copy pending payment-URI address/memo with snprintf("%s") instead of strncpy(…, sizeof-1), which does not NUL-terminate on truncation (F8-2). All build clean (full-node + tests) and the suite passes. Co-Authored-By: Claude Fable 5 --- src/ui/windows/console_input_model.cpp | 4 +++- src/ui/windows/export_transactions_dialog.cpp | 21 +++++++++++++------ src/ui/windows/send_tab.cpp | 5 +++-- tests/test_phase4.cpp | 6 ++++++ 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/ui/windows/console_input_model.cpp b/src/ui/windows/console_input_model.cpp index ee9fc91..646395d 100644 --- a/src/ui/windows/console_input_model.cpp +++ b/src/ui/windows/console_input_model.cpp @@ -127,7 +127,9 @@ std::vector ParseConsoleCommandArgs(const std::string& command) if (index >= command.size()) break; std::string token; + bool quoted = false; if (command[index] == '"' || command[index] == '\'') { + quoted = true; // an explicit "" is a real empty argument, not a gap to skip char quote = command[index++]; while (index < command.size() && command[index] != quote) { token += command[index++]; @@ -148,7 +150,7 @@ std::vector ParseConsoleCommandArgs(const std::string& command) token += command[index++]; } } - if (!token.empty()) args.push_back(token); + if (!token.empty() || quoted) args.push_back(token); } return args; } diff --git a/src/ui/windows/export_transactions_dialog.cpp b/src/ui/windows/export_transactions_dialog.cpp index e1b8b1e..084e413 100644 --- a/src/ui/windows/export_transactions_dialog.cpp +++ b/src/ui/windows/export_transactions_dialog.cpp @@ -29,21 +29,30 @@ static std::string s_status; // Helper to escape CSV field static std::string escapeCSV(const std::string& field) { - if (field.find(',') != std::string::npos || - field.find('"') != std::string::npos || - field.find('\n') != std::string::npos) { + // Neutralize spreadsheet formula injection: a field beginning with '=', '+', '-', '@', + // tab, or CR is interpreted as a formula by Excel/LibreOffice, letting an attacker-supplied + // memo/address execute on open. Prefix such fields with a single quote so they render as text. + std::string safe = field; + if (!safe.empty()) { + const char c0 = safe.front(); + if (c0 == '=' || c0 == '+' || c0 == '-' || c0 == '@' || c0 == '\t' || c0 == '\r') + safe.insert(safe.begin(), '\''); + } + if (safe.find(',') != std::string::npos || + safe.find('"') != std::string::npos || + safe.find('\n') != std::string::npos) { // Escape quotes and wrap in quotes std::string escaped; - escaped.reserve(field.size() + 4); + escaped.reserve(safe.size() + 4); escaped += '"'; - for (char c : field) { + for (char c : safe) { if (c == '"') escaped += "\"\""; else escaped += c; } escaped += '"'; return escaped; } - return field; + return safe; } void ExportTransactionsDialog::show() diff --git a/src/ui/windows/send_tab.cpp b/src/ui/windows/send_tab.cpp index 34bedb0..90cade6 100644 --- a/src/ui/windows/send_tab.cpp +++ b/src/ui/windows/send_tab.cpp @@ -1110,9 +1110,10 @@ void RenderSendTab(App* app) // Handle pending payment from URI if (app->hasPendingPayment()) { - strncpy(s_to_address, app->getPendingToAddress().c_str(), sizeof(s_to_address) - 1); + // snprintf always NUL-terminates; strncpy(…, sizeof-1) does not on truncation. + snprintf(s_to_address, sizeof(s_to_address), "%s", app->getPendingToAddress().c_str()); s_amount = app->getPendingAmount(); - strncpy(s_memo, app->getPendingMemo().c_str(), sizeof(s_memo) - 1); + snprintf(s_memo, sizeof(s_memo), "%s", app->getPendingMemo().c_str()); app->clearPendingPayment(); } diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 2bb4afd..eb6e319 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -2378,6 +2378,12 @@ void testConsoleInputModel() auto args = dragonx::ui::ParseConsoleCommandArgs("z_sendmany \"from addr\" [{\"address\":\"zs\",\"amount\":1.25}] true"); EXPECT_EQ(args.size(), static_cast(4)); EXPECT_EQ(args[1], std::string("from addr")); + // An explicitly-quoted empty argument must be preserved (not dropped), otherwise + // later positional RPC params silently shift by one (F14-1). + auto emptyArgs = dragonx::ui::ParseConsoleCommandArgs("walletpassphrase \"\" 60"); + EXPECT_EQ(emptyArgs.size(), static_cast(3)); + EXPECT_EQ(emptyArgs[1], std::string("")); + EXPECT_EQ(emptyArgs[2], std::string("60")); auto call = dragonx::ui::BuildConsoleRpcCall("setgenerate true 4"); EXPECT_TRUE(call.valid); EXPECT_EQ(call.method, std::string("setgenerate"));