ArgsManager: automate checking for correct command options
What changed, and why it matters
This commit adds an automated check in Bitcoin Core's argument manager so that command-line options meant only for specific wallet-tool commands (like -dumpfile) are rejected when used with the wrong command. It replaces a one-off manual check with a general mechanism and adds a test. It is a hardening improvement, not a fix for an active vulnerability.
No immediate action required. Treat as routine hardening. Review whether other tools or commands using ArgsManager should adopt CheckCommandOptions() for consistent validation.
Security signals we found
Input validation hardening for wallet tool command-line options
Replacement of manual option check with centralized ArgsManager validation
Addition of functional test for invalid option/command combination
Evidence from the diff
The change introduces ArgsManager::CheckCommandOptions(), which iterates over OptionsCategory::COMMAND_OPTIONS and verifies each specified option is registered for the given command via m_command_args. It then replaces the ad-hoc -dumpfile validation in ExecuteWalletToolFunc() with a call to this new method, and adds a functional test asserting that -dumpfile with the ‘create’ command is rejected. No other command-specific options appear to be newly restricted beyond what the previous manual check already covered.
Changed components
src/common/args.cppsrc/common/args.hsrc/wallet/wallettool.cpptest/functional/tool_wallet.pyInspect captured patch +51 / −3
diff --git a/src/common/args.cpp b/src/common/args.cpp
index 39dbbce0..e153d886 100644
--- a/src/common/args.cpp
+++ b/src/common/args.cpp
@@ -387,6 +387,40 @@ std::optional<const ArgsManager::Command> ArgsManager::GetCommand() const
return ret;
}
+bool ArgsManager::CheckCommandOptions(const std::string& command, std::vector<std::string>* errors) const
+{
+ LOCK(cs_args);
+
+ auto command_options = m_available_args.find(OptionsCategory::COMMAND_OPTIONS);
+ if (command_options == m_available_args.end()) {
+ // There are no command-specific options at all, so everything is fine
+ return true;
+ }
+
+ const auto command_args = m_command_args.find(command);
+ auto is_valid_opt = [&](const auto& opt) EXCLUSIVE_LOCKS_REQUIRED(cs_args) -> bool {
+ if (command_args == m_command_args.end()) {
+ // Caller may not have checked that command actually exists
+ // before calling this function. In that case, treat it as
+ // having no valid command-specific options.
+ return false;
+ } else {
+ return command_args->second.contains(opt);
+ }
+ };
+
+ bool ok = true;
+ for (const auto& [arg, _] : command_options->second) {
+ if (!GetSetting_(arg).isNull() && !is_valid_opt(arg)) {
+ ok = false;
+ if (errors != nullptr) {
+ errors->emplace_back(strprintf("The %s option cannot be used with the '%s' command.", arg, command));
+ }
+ }
+ }
+ return ok;
+}
+
std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
{
std::vector<std::string> result;
diff --git a/src/common/args.h b/src/common/args.h
index 766b2b22..0a3195f8 100644
--- a/src/common/args.h
+++ b/src/common/args.h
@@ -225,6 +225,16 @@ public:
*/
std::optional<const Command> GetCommand() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
+ /**
+ * Check that any command-specific options the user specified are valid
+ * for the given command.
+ *
+ * @param[in] command The command being run.
+ * @param[out] errors If non-null, populated with a message for each invalid option.
+ * @return false if any command-specific options were specified that are not valid for this command
+ */
+ bool CheckCommandOptions(const std::string& command, std::vector<std::string>* errors = nullptr) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
+
/**
* Get blocks directory path
*
diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp
index 7c24c0e1..6baab1f3 100644
--- a/src/wallet/wallettool.cpp
+++ b/src/wallet/wallettool.cpp
@@ -93,9 +93,12 @@ static void WalletShowInfo(CWallet* wallet_instance)
bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
{
- if (args.IsArgSet("-dumpfile") && command != "dump" && command != "createfromdump") {
- tfm::format(std::cerr, "The -dumpfile option can only be used with the \"dump\" and \"createfromdump\" commands.\n");
- return false;
+ {
+ std::vector<std::string> details;
+ if (!args.CheckCommandOptions(command, &details)) {
+ tfm::format(std::cerr, "Error: Invalid arguments provided:\n%s\n", util::MakeUnorderedList(details));
+ return false;
+ }
}
if ((command == "create" || command == "createfromdump") && !args.IsArgSet("-wallet")) {
tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n");
diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py
index 737aaf54..845d7a9a 100755
--- a/test/functional/tool_wallet.py
+++ b/test/functional/tool_wallet.py
@@ -421,6 +421,7 @@ class ToolWalletTest(BitcoinTestFramework):
assert not (self.nodes[0].wallets_path / "legacy").exists()
self.assert_raises_tool_error("Invalid parameter -descriptors", "-wallet=legacy", "-descriptors=false", "create")
assert not (self.nodes[0].wallets_path / "legacy").exists()
+ self.assert_raises_tool_error("The -dumpfile option cannot be used with the 'create' command.", "-wallet=legacy", "-dumpfile=wallet.dump", "create")
def test_no_create_unnamed(self):
self.log.info("Test that unnamed (default) wallets cannot be created")
Why this scored 28/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.