Skip to content

Commit 063b2c9

Browse files
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>
1 parent 8260578 commit 063b2c9

File tree

8 files changed

+98
-7
lines changed

8 files changed

+98
-7
lines changed

fuzz/cli11_app_fuzz.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
2929

3030
try {
3131
if(pstring_start > 0) {
32-
app->parse(parseString.substr(pstring_start, std::string::npos));
32+
app->parse(parseString.substr(pstring_start));
3333
} else {
3434
app->parse(parseString);
3535
}

include/CLI/impl/Config_inl.hpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -542,8 +542,9 @@ ConfigBase::to_config(const App *app, bool default_also, bool write_description,
542542
continue;
543543
}
544544

545-
std::string value = detail::ini_join(
546-
opt->reduced_results(), arraySeparator, arrayStart, arrayEnd, stringQuote, literalQuote);
545+
auto results = opt->reduced_results();
546+
std::string value =
547+
detail::ini_join(results, arraySeparator, arrayStart, arrayEnd, stringQuote, literalQuote);
547548

548549
bool isDefault = false;
549550
if(value.empty() && default_also) {
@@ -560,7 +561,16 @@ ConfigBase::to_config(const App *app, bool default_also, bool write_description,
560561
}
561562

562563
if(!value.empty()) {
563-
564+
if(opt->get_expected_max() > 1 && detail::is_binary_escaped_string(value) && results.size() == 1 &&
565+
!results[0].empty()) {
566+
if(results[0].front() == '[' && results[0].back() == ']') {
567+
// this is a condition which could be misinterpreted
568+
results[0].insert(0, 1, results[0].front());
569+
results[0].push_back(results[0].back());
570+
value = detail::ini_join(
571+
results, arraySeparator, arrayStart, arrayEnd, stringQuote, literalQuote);
572+
}
573+
}
564574
if(!opt->get_fnames().empty()) {
565575
try {
566576
value = opt->get_flag_value(single_name, value);

include/CLI/impl/Option_inl.hpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -665,13 +665,20 @@ CLI11_INLINE int Option::_add_result(std::string &&result, std::vector<std::stri
665665
if((allow_extra_args_ || get_expected_max() > 1) && !result.empty() && result.front() == '[' &&
666666
result.back() == ']') { // this is now a vector string likely from the default or user entry
667667
result.pop_back();
668-
669-
for(auto &var : CLI::detail::split_up(result.substr(1), ',')) {
668+
result.erase(result.begin());
669+
bool skipSection{false};
670+
for(auto &var : CLI::detail::split_up(result, ',')) {
671+
if(var == result) {
672+
skipSection = true;
673+
break;
674+
}
670675
if(!var.empty()) {
671676
result_count += _add_result(std::move(var), res);
672677
}
673678
}
674-
return result_count;
679+
if(!skipSection) {
680+
return result_count;
681+
}
675682
}
676683
if(delimiter_ == '\0') {
677684
res.push_back(std::move(result));

include/CLI/impl/StringTools_inl.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,13 @@ CLI11_INLINE std::string binary_escape_string(const std::string &string_to_escap
433433
stream << std::hex << static_cast<unsigned int>(static_cast<unsigned char>(c));
434434
std::string code = stream.str();
435435
escaped_string += std::string("\\x") + (code.size() < 2 ? "0" : "") + code;
436+
} else if(c == 'x' || c == 'X') {
437+
// need to check for inadvertent binary sequences
438+
if(!escaped_string.empty() && escaped_string.back() == '\\') {
439+
escaped_string += std::string("\\x") + (c == 'x' ? "78" : "58");
440+
} else {
441+
escaped_string.push_back(c);
442+
}
436443

437444
} else {
438445
escaped_string.push_back(c);

tests/FuzzFailTest.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,3 +260,33 @@ TEST_CASE("fuzz_config_test1") {
260260
CHECK(app->get_option_no_throw("--new_flag") != nullptr);
261261
CHECK(app->get_option_no_throw("--new_vector") != nullptr);
262262
}
263+
264+
// this test uses the same tests as above just with a full roundtrip test
265+
TEST_CASE("app_roundtrip_custom") {
266+
CLI::FuzzApp fuzzdata;
267+
CLI::FuzzApp fuzzdata2;
268+
auto app = fuzzdata.generateApp();
269+
auto app2 = fuzzdata2.generateApp();
270+
int index = GENERATE(range(1, 3));
271+
std::string optionString, flagString;
272+
auto parseData = loadFailureFile("round_trip_custom", index);
273+
std::size_t pstring_start{0};
274+
pstring_start = fuzzdata.add_custom_options(app.get(), parseData);
275+
276+
if(pstring_start > 0) {
277+
app->parse(parseData.substr(pstring_start));
278+
} else {
279+
app->parse(parseData);
280+
}
281+
282+
// should be able to write the config to a file and read from it again
283+
std::string configOut = app->config_to_str();
284+
app->clear();
285+
std::stringstream out(configOut);
286+
if(pstring_start > 0) {
287+
fuzzdata2.add_custom_options(app2.get(), parseData);
288+
}
289+
app2->parse_from_stream(out);
290+
auto result = fuzzdata2.compare(fuzzdata);
291+
CHECK(result);
292+
}

tests/HelpersTest.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,42 @@ TEST_CASE("StringTools: binaryEscapseConversion2", "[helpers]") {
301301
CHECK(rstring == testString);
302302
}
303303

304+
TEST_CASE("StringTools: binaryEscapseConversion_withX", "[helpers]") {
305+
std::string testString("hippy\\x35mm\\XF3_helpX26fox19");
306+
testString.push_back(0);
307+
testString.push_back(0);
308+
testString.push_back(0);
309+
testString.push_back(56);
310+
testString.push_back(-112);
311+
testString.push_back(-112);
312+
testString.push_back(39);
313+
testString.push_back(97);
314+
std::string estring = CLI::detail::binary_escape_string(testString);
315+
CHECK(CLI::detail::is_binary_escaped_string(estring));
316+
std::string rstring = CLI::detail::extract_binary_string(estring);
317+
CHECK(rstring == testString);
318+
}
319+
320+
TEST_CASE("StringTools: binaryEscapseConversion_withBrackets", "[helpers]") {
321+
322+
std::string vstr = R"raw('B"([\xb0\x0a\xb0/\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0])"')raw";
323+
std::string testString("[");
324+
testString.push_back(-80);
325+
testString.push_back('\n');
326+
testString.push_back(-80);
327+
testString.push_back('/');
328+
for(int ii = 0; ii < 13; ++ii) {
329+
testString.push_back(-80);
330+
}
331+
testString.push_back(']');
332+
333+
std::string estring = CLI::detail::binary_escape_string(testString);
334+
CHECK(CLI::detail::is_binary_escaped_string(estring));
335+
CHECK(estring == vstr);
336+
std::string rstring = CLI::detail::extract_binary_string(estring);
337+
CHECK(rstring == testString);
338+
}
339+
304340
TEST_CASE("StringTools: binaryStrings", "[helpers]") {
305341
std::string rstring = "B\"()\"";
306342
CHECK(CLI::detail::extract_binary_string(rstring).empty());

tests/fuzzFail/round_trip_custom1

39 Bytes
Binary file not shown.

tests/fuzzFail/round_trip_custom2

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--vM=[��/�������������]

0 commit comments

Comments
 (0)