From 063b2c911cea8bd4b908d6187c6a007ad320cb1a Mon Sep 17 00:00:00 2001 From: Philip Top Date: Sat, 30 Nov 2024 07:13:49 -0800 Subject: [PATCH] Fuzz fail (#1097) fix failing fuzz case involving binary string with a '\x' in it. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- fuzz/cli11_app_fuzz.cpp | 2 +- include/CLI/impl/Config_inl.hpp | 16 +++++++++--- include/CLI/impl/Option_inl.hpp | 13 +++++++--- include/CLI/impl/StringTools_inl.hpp | 7 ++++++ tests/FuzzFailTest.cpp | 30 ++++++++++++++++++++++ tests/HelpersTest.cpp | 36 +++++++++++++++++++++++++++ tests/fuzzFail/round_trip_custom1 | Bin 0 -> 39 bytes tests/fuzzFail/round_trip_custom2 | 1 + 8 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 tests/fuzzFail/round_trip_custom1 create mode 100644 tests/fuzzFail/round_trip_custom2 diff --git a/fuzz/cli11_app_fuzz.cpp b/fuzz/cli11_app_fuzz.cpp index 8a4e5f04..67d65bd7 100644 --- a/fuzz/cli11_app_fuzz.cpp +++ b/fuzz/cli11_app_fuzz.cpp @@ -29,7 +29,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { try { if(pstring_start > 0) { - app->parse(parseString.substr(pstring_start, std::string::npos)); + app->parse(parseString.substr(pstring_start)); } else { app->parse(parseString); } diff --git a/include/CLI/impl/Config_inl.hpp b/include/CLI/impl/Config_inl.hpp index 4979d874..040e7b52 100644 --- a/include/CLI/impl/Config_inl.hpp +++ b/include/CLI/impl/Config_inl.hpp @@ -542,8 +542,9 @@ ConfigBase::to_config(const App *app, bool default_also, bool write_description, continue; } - std::string value = detail::ini_join( - opt->reduced_results(), arraySeparator, arrayStart, arrayEnd, stringQuote, literalQuote); + auto results = opt->reduced_results(); + std::string value = + detail::ini_join(results, arraySeparator, arrayStart, arrayEnd, stringQuote, literalQuote); bool isDefault = false; if(value.empty() && default_also) { @@ -560,7 +561,16 @@ 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 ae955daf..b94453f9 100644 --- a/include/CLI/impl/Option_inl.hpp +++ b/include/CLI/impl/Option_inl.hpp @@ -665,13 +665,20 @@ CLI11_INLINE int Option::_add_result(std::string &&result, std::vector 1) && !result.empty() && result.front() == '[' && result.back() == ']') { // this is now a vector string likely from the default or user entry result.pop_back(); - - for(auto &var : CLI::detail::split_up(result.substr(1), ',')) { + 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); } } - return result_count; + if(!skipSection) { + return result_count; + } } if(delimiter_ == '\0') { res.push_back(std::move(result)); diff --git a/include/CLI/impl/StringTools_inl.hpp b/include/CLI/impl/StringTools_inl.hpp index 1dffb126..7fedfd5f 100644 --- a/include/CLI/impl/StringTools_inl.hpp +++ b/include/CLI/impl/StringTools_inl.hpp @@ -433,6 +433,13 @@ CLI11_INLINE std::string binary_escape_string(const std::string &string_to_escap stream << std::hex << static_cast(static_cast(c)); std::string code = stream.str(); escaped_string += std::string("\\x") + (code.size() < 2 ? "0" : "") + code; + } else if(c == 'x' || c == 'X') { + // need to check for inadvertent binary sequences + if(!escaped_string.empty() && escaped_string.back() == '\\') { + escaped_string += std::string("\\x") + (c == 'x' ? "78" : "58"); + } else { + escaped_string.push_back(c); + } } else { escaped_string.push_back(c); diff --git a/tests/FuzzFailTest.cpp b/tests/FuzzFailTest.cpp index a3ecdd49..852d9c6a 100644 --- a/tests/FuzzFailTest.cpp +++ b/tests/FuzzFailTest.cpp @@ -260,3 +260,33 @@ TEST_CASE("fuzz_config_test1") { CHECK(app->get_option_no_throw("--new_flag") != nullptr); CHECK(app->get_option_no_throw("--new_vector") != nullptr); } + +// this test uses the same tests as above just with a full roundtrip test +TEST_CASE("app_roundtrip_custom") { + CLI::FuzzApp fuzzdata; + CLI::FuzzApp fuzzdata2; + auto app = fuzzdata.generateApp(); + auto app2 = fuzzdata2.generateApp(); + int index = GENERATE(range(1, 3)); + std::string optionString, flagString; + auto parseData = loadFailureFile("round_trip_custom", index); + std::size_t pstring_start{0}; + pstring_start = fuzzdata.add_custom_options(app.get(), parseData); + + if(pstring_start > 0) { + app->parse(parseData.substr(pstring_start)); + } else { + app->parse(parseData); + } + + // should be able to write the config to a file and read from it again + std::string configOut = app->config_to_str(); + app->clear(); + std::stringstream out(configOut); + if(pstring_start > 0) { + fuzzdata2.add_custom_options(app2.get(), parseData); + } + app2->parse_from_stream(out); + auto result = fuzzdata2.compare(fuzzdata); + CHECK(result); +} diff --git a/tests/HelpersTest.cpp b/tests/HelpersTest.cpp index 65a19bf5..14bab00b 100644 --- a/tests/HelpersTest.cpp +++ b/tests/HelpersTest.cpp @@ -301,6 +301,42 @@ TEST_CASE("StringTools: binaryEscapseConversion2", "[helpers]") { CHECK(rstring == testString); } +TEST_CASE("StringTools: binaryEscapseConversion_withX", "[helpers]") { + std::string testString("hippy\\x35mm\\XF3_helpX26fox19"); + testString.push_back(0); + testString.push_back(0); + testString.push_back(0); + testString.push_back(56); + testString.push_back(-112); + testString.push_back(-112); + testString.push_back(39); + testString.push_back(97); + std::string estring = CLI::detail::binary_escape_string(testString); + CHECK(CLI::detail::is_binary_escaped_string(estring)); + std::string rstring = CLI::detail::extract_binary_string(estring); + CHECK(rstring == testString); +} + +TEST_CASE("StringTools: binaryEscapseConversion_withBrackets", "[helpers]") { + + std::string vstr = R"raw('B"([\xb0\x0a\xb0/\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0])"')raw"; + std::string testString("["); + testString.push_back(-80); + testString.push_back('\n'); + testString.push_back(-80); + testString.push_back('/'); + for(int ii = 0; ii < 13; ++ii) { + testString.push_back(-80); + } + testString.push_back(']'); + + std::string estring = CLI::detail::binary_escape_string(testString); + CHECK(CLI::detail::is_binary_escaped_string(estring)); + CHECK(estring == vstr); + std::string rstring = CLI::detail::extract_binary_string(estring); + CHECK(rstring == testString); +} + TEST_CASE("StringTools: binaryStrings", "[helpers]") { std::string rstring = "B\"()\""; CHECK(CLI::detail::extract_binary_string(rstring).empty()); diff --git a/tests/fuzzFail/round_trip_custom1 b/tests/fuzzFail/round_trip_custom1 new file mode 100644 index 0000000000000000000000000000000000000000..be58f611ab190dc169b79cb1fc7f8af8c229a379 GIT binary patch literal 39 rcmdPZEpz5FF)4H8D$6e@F;X`%F)@k(0%ZmU1|St-lok^sVPpUR%6bX_ literal 0 HcmV?d00001 diff --git a/tests/fuzzFail/round_trip_custom2 b/tests/fuzzFail/round_trip_custom2 new file mode 100644 index 00000000..9e5692cb --- /dev/null +++ b/tests/fuzzFail/round_trip_custom2 @@ -0,0 +1 @@ +--vM=[° °/°°°°°°°°°°°°°] \ No newline at end of file