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
This commit is contained in:
fanquake 2021-05-24 10:59:51 +08:00 committed by PastaPastaPasta
parent 0903c3bb5b
commit 807720666c
2 changed files with 12 additions and 7 deletions

View File

@ -353,14 +353,12 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
} }
// we do not allow -includeconf from command line // we do not allow -includeconf from command line
bool success = true;
if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) {
for (const auto& include : util::SettingsSpan(*includes)) { const auto& include{*util::SettingsSpan(*includes).begin()}; // pick first value as example
error += "-includeconf cannot be used from commandline; -includeconf=" + include.get_str() + "\n"; error = "-includeconf cannot be used from commandline; -includeconf=" + include.write();
success = false; return false;
} }
} return true;
return success;
} }
std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) const std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) const

View File

@ -43,7 +43,14 @@ class IncludeConfTest(BitcoinTestFramework):
self.log.info("-includeconf cannot be used as command-line arg") self.log.info("-includeconf cannot be used as command-line arg")
self.stop_node(0) 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)/'") 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: with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f: