From 807720666cffab51857b4c70f0a66e80b5fba9ec Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 24 May 2021 10:59:51 +0800 Subject: [PATCH] Merge bitcoin/bitcoin#22002: Fix crash when parsing command line with -noincludeconf=0 fad0867d6ab9430070aa7d60bf7617a6508e0586 Cleanup -includeconf error message (MarcoFalke) fa9f711c3746ca3962f15224285a453744cd45b3 Fix crash when parsing command line with -noincludeconf=0 (MarcoFalke) Pull request description: The error message has several issues: * It may crash instead of cleanly shutting down, when `-noincludeconf=0` is passed * It doesn't quote the value * It includes an erroneous trailing `\n` * It is redundantly mentioning `"-includeconf cannot be used from commandline;"` several times, when once should be more than sufficient Fix all issues by: * Replacing `get_str()` with `write()` to fix the crash and quoting issue * Remove the `\n` and only print the first value to fix the other issues Before: ``` $ ./src/bitcoind -noincludeconf=0 terminate called after throwing an instance of 'std::runtime_error' what(): JSON value is not a string as expected Aborted (core dumped) $ ./src/bitcoind -includeconf='a b' -includeconf=c Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=a b -includeconf cannot be used from commandline; -includeconf=c ``` After: ``` $ ./src/bitcoind -noincludeconf=0 Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=true $ ./src/bitcoind -includeconf='a b' -includeconf=c Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf="a b" ``` Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34493 Testcase: https://github.com/bitcoin/bitcoin/files/6515429/clusterfuzz-testcase-minimized-system-6328535926046720.log ``` FUZZ=system ./src/test/fuzz/fuzz ./clusterfuzz-testcase-minimized-system-6328535926046720.log ``` See https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md ACKs for top commit: sipa: utACK fad0867d6ab9430070aa7d60bf7617a6508e0586 Tree-SHA512: b44af93be6bf71b43669058c1449c4c6999f03b5b01b429851b149b12d77733408cb207e9a3edc6f0bffd6030c4c52165e8e23a1c2718ff5082a6ba254cc94a4 --- src/util/system.cpp | 10 ++++------ test/functional/feature_includeconf.py | 9 ++++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/util/system.cpp b/src/util/system.cpp index 915deb8e97..2685121761 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -353,14 +353,12 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin } // we do not allow -includeconf from command line - bool success = true; if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { - for (const auto& include : util::SettingsSpan(*includes)) { - error += "-includeconf cannot be used from commandline; -includeconf=" + include.get_str() + "\n"; - success = false; - } + const auto& include{*util::SettingsSpan(*includes).begin()}; // pick first value as example + error = "-includeconf cannot be used from commandline; -includeconf=" + include.write(); + return false; } - return success; + return true; } std::optional ArgsManager::GetArgFlags(const std::string& name) const diff --git a/test/functional/feature_includeconf.py b/test/functional/feature_includeconf.py index ab93b13734..879bb54a94 100755 --- a/test/functional/feature_includeconf.py +++ b/test/functional/feature_includeconf.py @@ -43,7 +43,14 @@ class IncludeConfTest(BitcoinTestFramework): self.log.info("-includeconf cannot be used as command-line arg") self.stop_node(0) - self.nodes[0].assert_start_raises_init_error(extra_args=["-includeconf=relative2.conf"], expected_msg="Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=relative2.conf") + self.nodes[0].assert_start_raises_init_error( + extra_args=['-noincludeconf=0'], + expected_msg='Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=true', + ) + self.nodes[0].assert_start_raises_init_error( + extra_args=['-includeconf=relative2.conf', '-includeconf=no_warn.conf'], + expected_msg='Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf="relative2.conf"', + ) self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'") with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f: