Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix a fuzzing issue from a string as a bracket #1110

Merged
merged 10 commits into from
Jan 3, 2025
Merged
20 changes: 20 additions & 0 deletions book/chapters/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions include/CLI/impl/Config_inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
29 changes: 25 additions & 4 deletions include/CLI/impl/Option_inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,16 +662,37 @@

CLI11_INLINE int Option::_add_result(std::string &&result, std::vector<std::string> &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]);

Check warning on line 675 in include/CLI/impl/Option_inl.hpp

View check run for this annotation

Codecov / codecov/patch

include/CLI/impl/Option_inl.hpp#L675

Added line #L675 was not covered by tests
} 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);
}
Expand Down
15 changes: 15 additions & 0 deletions include/CLI/impl/StringTools_inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -519,23 +519,38 @@
}
}

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{"[["};

Check warning on line 525 in include/CLI/impl/StringTools_inl.hpp

View check run for this annotation

Codecov / codecov/patch

include/CLI/impl/StringTools_inl.hpp#L525

Added line #L525 was not covered by tests
for(std::size_t ii = 1; ii < str.size(); ++ii) {
tstr.push_back(str[ii]);
tstr.push_back(str[ii]);

Check warning on line 528 in include/CLI/impl/StringTools_inl.hpp

View check run for this annotation

Codecov / codecov/patch

include/CLI/impl/StringTools_inl.hpp#L527-L528

Added lines #L527 - L528 were not covered by tests
}
str = std::move(tstr);
}

Check warning on line 531 in include/CLI/impl/StringTools_inl.hpp

View check run for this annotation

Codecov / codecov/patch

include/CLI/impl/StringTools_inl.hpp#L530-L531

Added lines #L530 - L531 were not covered by tests
}

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) {
detail::remove_outer(str, string_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;
Expand Down
2 changes: 1 addition & 1 deletion tests/FuzzFailTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
27 changes: 27 additions & 0 deletions tests/OptionTypeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,21 @@ TEST_CASE_METHOD(TApp, "vectorSingleArg", "[optiontype]") {
CHECK("4" == extra);
}

TEST_CASE_METHOD(TApp, "vectorEmptyArg", "[optiontype]") {

std::vector<std::string> 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<std::pair<int, std::string>> cv;
Expand All @@ -1249,6 +1264,18 @@ TEST_CASE_METHOD(TApp, "vectorEmpty", "[optiontype]") {
CHECK(cv.empty());
}

TEST_CASE_METHOD(TApp, "vectorVectorArg", "[optiontype]") {

std::vector<std::vector<std::string>> 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};
Expand Down
Binary file added tests/fuzzFail/round_trip_custom3
Binary file not shown.
Loading