What changed, and why it matters
This commit changes the 'bitcoin' wrapper program so it automatically picks the multi-process version of Bitcoin tools when certain IPC options are present, instead of requiring users to manually pass -m. It is a usability improvement for an experimental multi-process build path. There is no direct security vulnerability in the diff, but it slightly increases the attack surface reachable through the wrapper by making multi-process binaries easier to invoke.
No immediate action required. Treat as a normal feature/usability commit. If auditing, verify that the wrapper's pre-parse of IPC options cannot be abused to bypass intended binary selection or leak sensitive config parsing errors.
Security signals we found
New argument parsing path added to wrapper binary
ArgsManager configured to accept unknown arguments (ALLOW_ANY) in wrapper pre-parse
Wrapper now auto-selects multiprocess binaries based on user-supplied IPC options
CMake links wrapper against additional library (bitcoin_common)
Evidence from the diff
The wrapper binary now parses command-line and config-file arguments using ArgsManager to detect -ipcbind, -ipcconnect, or -ipcfd. If any are set, it launches bitcoin-node/bitcoin-gui instead of bitcoind/bitcoin-qt, even without -m. ArgsManager is extended with SetDefaultFlags(ALLOW_ANY) so unknown arguments are accepted during this pre-flight parse. The wrapper also links against bitcoin_common. Tests are updated to expect the new behavior and to allow undocumented -ipcconnect/-ipcfd options.
Changed components
src/bitcoin.cppsrc/common/args.cppsrc/common/args.hsrc/CMakeLists.txttest/functional/tool_bitcoin.pyInspect captured patch +58 / −11
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 9abbf958..f9934bb5 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -292,7 +292,7 @@ if(BUILD_BITCOIN_BIN)
add_executable(bitcoin bitcoin.cpp)
add_windows_resources(bitcoin bitcoin-res.rc)
add_windows_application_manifest(bitcoin)
- target_link_libraries(bitcoin core_interface bitcoin_util)
+ target_link_libraries(bitcoin core_interface bitcoin_common bitcoin_util)
install_binary_component(bitcoin HAS_MANPAGE)
endif()
diff --git a/src/bitcoin.cpp b/src/bitcoin.cpp
index c3278274..c1a5fce3 100644
--- a/src/bitcoin.cpp
+++ b/src/bitcoin.cpp
@@ -5,6 +5,7 @@
#include <bitcoin-build-config.h> // IWYU pragma: keep
#include <clientversion.h>
+#include <common/args.h>
#include <util/fs.h>
#include <util/exec.h>
#include <util/strencodings.h>
@@ -47,7 +48,7 @@ Run '%s help' to see additional commands (e.g. for testing and debugging).
)";
struct CommandLine {
- bool use_multiprocess{false};
+ std::optional<bool> use_multiprocess;
bool show_version{false};
bool show_help{false};
std::string_view command;
@@ -55,6 +56,7 @@ struct CommandLine {
};
CommandLine ParseCommandLine(int argc, char* argv[]);
+bool UseMultiprocess(const CommandLine& cmd);
static void ExecCommand(const std::vector<const char*>& args, std::string_view argv0);
int main(int argc, char* argv[])
@@ -78,9 +80,9 @@ int main(int argc, char* argv[])
return EXIT_FAILURE;
}
} else if (cmd.command == "gui") {
- args.emplace_back(cmd.use_multiprocess ? "bitcoin-gui" : "bitcoin-qt");
+ args.emplace_back(UseMultiprocess(cmd) ? "bitcoin-gui" : "bitcoin-qt");
} else if (cmd.command == "node") {
- args.emplace_back(cmd.use_multiprocess ? "bitcoin-node" : "bitcoind");
+ args.emplace_back(UseMultiprocess(cmd) ? "bitcoin-node" : "bitcoind");
} else if (cmd.command == "rpc") {
args.emplace_back("bitcoin-cli");
// Since "bitcoin rpc" is a new interface that doesn't need to be
@@ -143,6 +145,30 @@ CommandLine ParseCommandLine(int argc, char* argv[])
return cmd;
}
+bool UseMultiprocess(const CommandLine& cmd)
+{
+ // If -m or -M options were explicitly specified, there is no need to
+ // further parse arguments to determine which to use.
+ if (cmd.use_multiprocess) return *cmd.use_multiprocess;
+
+ ArgsManager args;
+ args.SetDefaultFlags(ArgsManager::ALLOW_ANY);
+ std::string error_message;
+ auto argv{cmd.args};
+ argv.insert(argv.begin(), nullptr);
+ if (!args.ParseParameters(argv.size(), argv.data(), error_message)) {
+ tfm::format(std::cerr, "Warning: failed to parse subcommand command line options: %s\n", error_message);
+ }
+ if (!args.ReadConfigFiles(error_message, true)) {
+ tfm::format(std::cerr, "Warning: failed to parse subcommand config: %s\n", error_message);
+ }
+ args.SelectConfigNetwork(args.GetChainTypeString());
+
+ // If any -ipc* options are set these need to be processed by a
+ // multiprocess-capable binary.
+ return args.IsArgSet("-ipcbind") || args.IsArgSet("-ipcconnect") || args.IsArgSet("-ipcfd");
+}
+
//! Execute the specified bitcoind, bitcoin-qt or other command line in `args`
//! using src, bin and libexec directory paths relative to this executable, where
//! the path to this executable is specified in `wrapper_argv0`.
diff --git a/src/common/args.cpp b/src/common/args.cpp
index ff30ec5b..d44cd431 100644
--- a/src/common/args.cpp
+++ b/src/common/args.cpp
@@ -266,7 +266,13 @@ std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) co
return search->second.m_flags;
}
}
- return std::nullopt;
+ return m_default_flags;
+}
+
+void ArgsManager::SetDefaultFlags(std::optional<unsigned int> flags)
+{
+ LOCK(cs_args);
+ m_default_flags = flags;
}
fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value) const
diff --git a/src/common/args.h b/src/common/args.h
index da19cbda..d907ad76 100644
--- a/src/common/args.h
+++ b/src/common/args.h
@@ -137,6 +137,7 @@ protected:
std::string m_network GUARDED_BY(cs_args);
std::set<std::string> m_network_only_args GUARDED_BY(cs_args);
std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
+ std::optional<unsigned int> m_default_flags GUARDED_BY(cs_args){};
bool m_accept_any_command GUARDED_BY(cs_args){true};
std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
std::optional<fs::path> m_config_path GUARDED_BY(cs_args);
@@ -375,10 +376,15 @@ protected:
/**
* Return Flags for known arg.
- * Return nullopt for unknown arg.
+ * Return default flags for unknown arg.
*/
std::optional<unsigned int> GetArgFlags(const std::string& name) const;
+ /**
+ * Set default flags to return for an unknown arg.
+ */
+ void SetDefaultFlags(std::optional<unsigned int>);
+
/**
* Get settings file path, or return false if read-write settings were
* disabled with -nosettings.
diff --git a/test/functional/tool_bitcoin.py b/test/functional/tool_bitcoin.py
index dcfdb2d7..1112e02e 100755
--- a/test/functional/tool_bitcoin.py
+++ b/test/functional/tool_bitcoin.py
@@ -7,7 +7,10 @@ from test_framework.test_framework import (
BitcoinTestFramework,
SkipTest,
)
-from test_framework.util import assert_equal
+from test_framework.util import (
+ append_config,
+ assert_equal,
+)
import platform
import re
@@ -55,12 +58,11 @@ class ToolBitcoinTest(BitcoinTestFramework):
raise RuntimeError(f"Unexpected output from {node.args + extra_args}: {out=!r} {err=!r} {ret=!r}") from e
def run_test(self):
+ node = self.nodes[0]
+
self.log.info("Ensure bitcoin node command invokes bitcoind by default")
self.test_args([], [], expect_exe="bitcoind")
- self.log.info("Ensure bitcoin command does not accept -ipcbind by default")
- self.test_args(["-M"], ["-ipcbind=unix"], expect_error='Error: Error parsing command line arguments: Invalid parameter -ipcbind=unix')
-
self.log.info("Ensure bitcoin -M invokes bitcoind")
self.test_args(["-M"], [], expect_exe="bitcoind")
@@ -74,6 +76,13 @@ class ToolBitcoinTest(BitcoinTestFramework):
self.log.info("Ensure bitcoin -m does accept -ipcbind")
self.test_args(["-m"], ["-ipcbind=unix"], expect_exe="bitcoin-node")
+ self.log.info("Ensure bitcoin accepts -ipcbind by default")
+ self.test_args([], ["-ipcbind=unix"], expect_exe="bitcoin-node")
+
+ self.log.info("Ensure bitcoin recognizes -ipcbind in config file")
+ append_config(node.datadir_path, ["ipcbind=unix"])
+ self.test_args([], [], expect_exe="bitcoin-node")
+
def get_node_output(node):
ret = node.process.wait(timeout=60)
diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py
index 3e9e5ba2..e47ff30f 100755
--- a/test/lint/check-doc.py
+++ b/test/lint/check-doc.py
@@ -23,7 +23,7 @@ CMD_GREP_WALLET_ARGS = r"git grep --function-context 'void WalletInit::AddWallet
CMD_GREP_WALLET_HIDDEN_ARGS = r"git grep --function-context 'void DummyWalletInit::AddWalletOptions' -- {}".format(CMD_ROOT_DIR)
CMD_GREP_DOCS = r"git grep --perl-regexp '{}' {}".format(REGEX_DOC, CMD_ROOT_DIR)
# list unsupported, deprecated and duplicate args as they need no documentation
-SET_DOC_OPTIONAL = set(['-h', '-?', '-dbcrashratio', '-forcecompactdb'])
+SET_DOC_OPTIONAL = set(['-h', '-?', '-dbcrashratio', '-forcecompactdb', '-ipcconnect', '-ipcfd'])
def lint_missing_argument_documentation():
Why this scored 18/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.