Skip to content

Commit f41e59b

Browse files
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>
1 parent d2b331f commit f41e59b

File tree

7 files changed

+88
-15
lines changed

7 files changed

+88
-15
lines changed

Diff for: book/chapters/config.md

+20
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,26 @@ The config parser has a modifier
259259
This modification will insert the separator between each line even if not
260260
sequential. This allows an input option to be configured with multiple lines.
261261

262+
```toml
263+
# Examples of vector of vector inputs in config
264+
265+
# this example is how config_to_str writes it out
266+
vector1 = [a,v,"[]"]
267+
```
268+
269+
The field insertion has a special processing for duplicate characters starting
270+
with "[[" in which case the `"[]"` gets translated to `[[]]` before getting
271+
passed into the option which converts it back into the correct string. This can
272+
also be used on the command line to handle unusual parsing situation with
273+
brackets.
274+
275+
### Argument With Brackets
276+
277+
There is an edge case with actual strings that are surrounded by brackets. For
278+
example if the string "[]" needed to be passed. this would normally trigger the
279+
bracket processing and result in an empty vector. In this case it can be
280+
enclosed in quotes and should be handled correctly.
281+
262282
## Multiple configuration files
263283

264284
If it is desired that multiple configuration be allowed. Use

Diff for: include/CLI/impl/Config_inl.hpp

-10
Original file line numberDiff line numberDiff line change
@@ -561,16 +561,6 @@ ConfigBase::to_config(const App *app, bool default_also, bool write_description,
561561
}
562562

563563
if(!value.empty()) {
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-
}
574564
if(!opt->get_fnames().empty()) {
575565
try {
576566
value = opt->get_flag_value(single_name, value);

Diff for: include/CLI/impl/Option_inl.hpp

+25-4
Original file line numberDiff line numberDiff line change
@@ -662,16 +662,37 @@ CLI11_INLINE std::string Option::_validate(std::string &result, int index) const
662662

663663
CLI11_INLINE int Option::_add_result(std::string &&result, std::vector<std::string> &res) const {
664664
int result_count = 0;
665+
666+
// Handle the vector escape possibility all characters duplicated and starting with [[ ending with ]]
667+
// this is always a single result
668+
if(result.size() >= 4 && result[0] == '[' && result[1] == '[' && result.back() == ']' &&
669+
(*(result.end() - 2) == ']')) {
670+
// this is an escape clause for odd strings
671+
std::string nstrs{'['};
672+
bool duplicated{true};
673+
for(std::size_t ii = 2; ii < result.size() - 2; ii += 2) {
674+
if(result[ii] == result[ii + 1]) {
675+
nstrs.push_back(result[ii]);
676+
} else {
677+
duplicated = false;
678+
break;
679+
}
680+
}
681+
if(duplicated) {
682+
nstrs.push_back(']');
683+
res.push_back(std::move(nstrs));
684+
++result_count;
685+
return result_count;
686+
}
687+
}
688+
665689
if((allow_extra_args_ || get_expected_max() > 1) && !result.empty() && result.front() == '[' &&
666690
result.back() == ']') { // this is now a vector string likely from the default or user entry
691+
667692
result.pop_back();
668693
result.erase(result.begin());
669694
bool skipSection{false};
670695
for(auto &var : CLI::detail::split_up(result, ',')) {
671-
if(var == result) {
672-
skipSection = true;
673-
break;
674-
}
675696
if(!var.empty()) {
676697
result_count += _add_result(std::move(var), res);
677698
}

Diff for: include/CLI/impl/StringTools_inl.hpp

+15
Original file line numberDiff line numberDiff line change
@@ -519,23 +519,38 @@ CLI11_INLINE void remove_quotes(std::vector<std::string> &args) {
519519
}
520520
}
521521

522+
CLI11_INLINE void handle_secondary_array(std::string &str) {
523+
if(str.size() >= 2 && str.front() == '[' && str.back() == ']') {
524+
// handle some special array processing for arguments if it might be interpreted as a secondary array
525+
std::string tstr{"[["};
526+
for(std::size_t ii = 1; ii < str.size(); ++ii) {
527+
tstr.push_back(str[ii]);
528+
tstr.push_back(str[ii]);
529+
}
530+
str = std::move(tstr);
531+
}
532+
}
533+
522534
CLI11_INLINE bool process_quoted_string(std::string &str, char string_char, char literal_char) {
523535
if(str.size() <= 1) {
524536
return false;
525537
}
526538
if(detail::is_binary_escaped_string(str)) {
527539
str = detail::extract_binary_string(str);
540+
handle_secondary_array(str);
528541
return true;
529542
}
530543
if(str.front() == string_char && str.back() == string_char) {
531544
detail::remove_outer(str, string_char);
532545
if(str.find_first_of('\\') != std::string::npos) {
533546
str = detail::remove_escaped_characters(str);
534547
}
548+
handle_secondary_array(str);
535549
return true;
536550
}
537551
if((str.front() == literal_char || str.front() == '`') && str.back() == str.front()) {
538552
detail::remove_outer(str, str.front());
553+
handle_secondary_array(str);
539554
return true;
540555
}
541556
return false;

Diff for: tests/FuzzFailTest.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ TEST_CASE("app_roundtrip_custom") {
267267
CLI::FuzzApp fuzzdata2;
268268
auto app = fuzzdata.generateApp();
269269
auto app2 = fuzzdata2.generateApp();
270-
int index = GENERATE(range(1, 3));
270+
int index = GENERATE(range(1, 4));
271271
std::string optionString, flagString;
272272
auto parseData = loadFailureFile("round_trip_custom", index);
273273
std::size_t pstring_start{0};

Diff for: tests/OptionTypeTest.cpp

+27
Original file line numberDiff line numberDiff line change
@@ -1225,6 +1225,21 @@ TEST_CASE_METHOD(TApp, "vectorSingleArg", "[optiontype]") {
12251225
CHECK("4" == extra);
12261226
}
12271227

1228+
TEST_CASE_METHOD(TApp, "vectorEmptyArg", "[optiontype]") {
1229+
1230+
std::vector<std::string> cv{"test"};
1231+
app.add_option("-c", cv);
1232+
args = {"-c", "test1", "[]"};
1233+
1234+
run();
1235+
CHECK(cv.size() == 1);
1236+
args = {"-c", "test1", "[[]]"};
1237+
1238+
run();
1239+
CHECK(cv.size() == 2);
1240+
CHECK(cv[1] == "[]");
1241+
}
1242+
12281243
TEST_CASE_METHOD(TApp, "vectorDoubleArg", "[optiontype]") {
12291244

12301245
std::vector<std::pair<int, std::string>> cv;
@@ -1249,6 +1264,18 @@ TEST_CASE_METHOD(TApp, "vectorEmpty", "[optiontype]") {
12491264
CHECK(cv.empty());
12501265
}
12511266

1267+
TEST_CASE_METHOD(TApp, "vectorVectorArg", "[optiontype]") {
1268+
1269+
std::vector<std::vector<std::string>> cv{};
1270+
app.add_option("-c", cv);
1271+
args = {"-c", "[[a,b]]"};
1272+
1273+
run();
1274+
CHECK(cv.size() == 1);
1275+
CHECK(cv[0].size() == 2);
1276+
CHECK(cv[0][0] == "a");
1277+
}
1278+
12521279
TEST_CASE_METHOD(TApp, "OnParseCall", "[optiontype]") {
12531280

12541281
int cnt{0};

Diff for: tests/fuzzFail/round_trip_custom3

31 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)