From f41e59b7f654b46de13a86d721fd6192e2872653 Mon Sep 17 00:00:00 2001 From: Philip Top Date: Fri, 3 Jan 2025 07:47:09 -0800 Subject: [PATCH] fix a fuzzing issue from a string as a bracket (#1110) fix fuzzing issue --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- book/chapters/config.md | 20 ++++++++++++++++++ include/CLI/impl/Config_inl.hpp | 10 --------- include/CLI/impl/Option_inl.hpp | 29 +++++++++++++++++++++++---- include/CLI/impl/StringTools_inl.hpp | 15 ++++++++++++++ tests/FuzzFailTest.cpp | 2 +- tests/OptionTypeTest.cpp | 27 +++++++++++++++++++++++++ tests/fuzzFail/round_trip_custom3 | Bin 0 -> 31 bytes 7 files changed, 88 insertions(+), 15 deletions(-) create mode 100644 tests/fuzzFail/round_trip_custom3 diff --git a/book/chapters/config.md b/book/chapters/config.md index a03f7a72d..d75d5fe8d 100644 --- a/book/chapters/config.md +++ b/book/chapters/config.md @@ -259,6 +259,26 @@ The config parser has a modifier This modification will insert the separator between each line even if not sequential. This allows an input option to be configured with multiple lines. +```toml +# Examples of vector of vector inputs in config + +# this example is how config_to_str writes it out +vector1 = [a,v,"[]"] +``` + +The field insertion has a special processing for duplicate characters starting +with "[[" in which case the `"[]"` gets translated to `[[]]` before getting +passed into the option which converts it back into the correct string. This can +also be used on the command line to handle unusual parsing situation with +brackets. + +### Argument With Brackets + +There is an edge case with actual strings that are surrounded by brackets. For +example if the string "[]" needed to be passed. this would normally trigger the +bracket processing and result in an empty vector. In this case it can be +enclosed in quotes and should be handled correctly. + ## Multiple configuration files If it is desired that multiple configuration be allowed. Use diff --git a/include/CLI/impl/Config_inl.hpp b/include/CLI/impl/Config_inl.hpp index 2485c8fc0..30c7241c1 100644 --- a/include/CLI/impl/Config_inl.hpp +++ b/include/CLI/impl/Config_inl.hpp @@ -561,16 +561,6 @@ ConfigBase::to_config(const App *app, bool default_also, bool write_description, } if(!value.empty()) { - if(opt->get_expected_max() > 1 && detail::is_binary_escaped_string(value) && results.size() == 1 && - !results[0].empty()) { - if(results[0].front() == '[' && results[0].back() == ']') { - // this is a condition which could be misinterpreted - results[0].insert(0, 1, results[0].front()); - results[0].push_back(results[0].back()); - value = detail::ini_join( - results, arraySeparator, arrayStart, arrayEnd, stringQuote, literalQuote); - } - } if(!opt->get_fnames().empty()) { try { value = opt->get_flag_value(single_name, value); diff --git a/include/CLI/impl/Option_inl.hpp b/include/CLI/impl/Option_inl.hpp index b94453f94..e39060b21 100644 --- a/include/CLI/impl/Option_inl.hpp +++ b/include/CLI/impl/Option_inl.hpp @@ -662,16 +662,37 @@ CLI11_INLINE std::string Option::_validate(std::string &result, int index) const CLI11_INLINE int Option::_add_result(std::string &&result, std::vector &res) const { int result_count = 0; + + // Handle the vector escape possibility all characters duplicated and starting with [[ ending with ]] + // this is always a single result + if(result.size() >= 4 && result[0] == '[' && result[1] == '[' && result.back() == ']' && + (*(result.end() - 2) == ']')) { + // this is an escape clause for odd strings + std::string nstrs{'['}; + bool duplicated{true}; + for(std::size_t ii = 2; ii < result.size() - 2; ii += 2) { + if(result[ii] == result[ii + 1]) { + nstrs.push_back(result[ii]); + } else { + duplicated = false; + break; + } + } + if(duplicated) { + nstrs.push_back(']'); + res.push_back(std::move(nstrs)); + ++result_count; + return result_count; + } + } + if((allow_extra_args_ || get_expected_max() > 1) && !result.empty() && result.front() == '[' && result.back() == ']') { // this is now a vector string likely from the default or user entry + result.pop_back(); result.erase(result.begin()); bool skipSection{false}; for(auto &var : CLI::detail::split_up(result, ',')) { - if(var == result) { - skipSection = true; - break; - } if(!var.empty()) { result_count += _add_result(std::move(var), res); } diff --git a/include/CLI/impl/StringTools_inl.hpp b/include/CLI/impl/StringTools_inl.hpp index 7fedfd5f3..76cc14a7a 100644 --- a/include/CLI/impl/StringTools_inl.hpp +++ b/include/CLI/impl/StringTools_inl.hpp @@ -519,12 +519,25 @@ CLI11_INLINE void remove_quotes(std::vector &args) { } } +CLI11_INLINE void handle_secondary_array(std::string &str) { + if(str.size() >= 2 && str.front() == '[' && str.back() == ']') { + // handle some special array processing for arguments if it might be interpreted as a secondary array + std::string tstr{"[["}; + for(std::size_t ii = 1; ii < str.size(); ++ii) { + tstr.push_back(str[ii]); + tstr.push_back(str[ii]); + } + str = std::move(tstr); + } +} + CLI11_INLINE bool process_quoted_string(std::string &str, char string_char, char literal_char) { if(str.size() <= 1) { return false; } if(detail::is_binary_escaped_string(str)) { str = detail::extract_binary_string(str); + handle_secondary_array(str); return true; } if(str.front() == string_char && str.back() == string_char) { @@ -532,10 +545,12 @@ CLI11_INLINE bool process_quoted_string(std::string &str, char string_char, char if(str.find_first_of('\\') != std::string::npos) { str = detail::remove_escaped_characters(str); } + handle_secondary_array(str); return true; } if((str.front() == literal_char || str.front() == '`') && str.back() == str.front()) { detail::remove_outer(str, str.front()); + handle_secondary_array(str); return true; } return false; diff --git a/tests/FuzzFailTest.cpp b/tests/FuzzFailTest.cpp index 852d9c6a6..38db12e6b 100644 --- a/tests/FuzzFailTest.cpp +++ b/tests/FuzzFailTest.cpp @@ -267,7 +267,7 @@ TEST_CASE("app_roundtrip_custom") { CLI::FuzzApp fuzzdata2; auto app = fuzzdata.generateApp(); auto app2 = fuzzdata2.generateApp(); - int index = GENERATE(range(1, 3)); + int index = GENERATE(range(1, 4)); std::string optionString, flagString; auto parseData = loadFailureFile("round_trip_custom", index); std::size_t pstring_start{0}; diff --git a/tests/OptionTypeTest.cpp b/tests/OptionTypeTest.cpp index 09dfa1d2f..6229eb48d 100644 --- a/tests/OptionTypeTest.cpp +++ b/tests/OptionTypeTest.cpp @@ -1225,6 +1225,21 @@ TEST_CASE_METHOD(TApp, "vectorSingleArg", "[optiontype]") { CHECK("4" == extra); } +TEST_CASE_METHOD(TApp, "vectorEmptyArg", "[optiontype]") { + + std::vector cv{"test"}; + app.add_option("-c", cv); + args = {"-c", "test1", "[]"}; + + run(); + CHECK(cv.size() == 1); + args = {"-c", "test1", "[[]]"}; + + run(); + CHECK(cv.size() == 2); + CHECK(cv[1] == "[]"); +} + TEST_CASE_METHOD(TApp, "vectorDoubleArg", "[optiontype]") { std::vector> cv; @@ -1249,6 +1264,18 @@ TEST_CASE_METHOD(TApp, "vectorEmpty", "[optiontype]") { CHECK(cv.empty()); } +TEST_CASE_METHOD(TApp, "vectorVectorArg", "[optiontype]") { + + std::vector> cv{}; + app.add_option("-c", cv); + args = {"-c", "[[a,b]]"}; + + run(); + CHECK(cv.size() == 1); + CHECK(cv[0].size() == 2); + CHECK(cv[0][0] == "a"); +} + TEST_CASE_METHOD(TApp, "OnParseCall", "[optiontype]") { int cnt{0}; diff --git a/tests/fuzzFail/round_trip_custom3 b/tests/fuzzFail/round_trip_custom3 new file mode 100644 index 0000000000000000000000000000000000000000..e7991992d99e3a0716c243b14cefad972eb4b805 GIT binary patch literal 31 jcmdPZEpz6ovf*$%#;p(?8w*6-(Lh3>r{{*&e+C8sktYf4 literal 0 HcmV?d00001