diff --git a/args.hxx b/args.hxx index 86a8672..42afa91 100644 --- a/args.hxx +++ b/args.hxx @@ -2949,7 +2949,20 @@ namespace args if (optionType == OptionType::LongFlag && allowJoinedLongValue) { const auto separator = longseparator.empty() ? chunk.npos : chunk.find(longseparator); - if (separator != chunk.npos) + // Only attempt joined-value completion when the + // separator lies at or past the long prefix, so + // there is a (possibly empty) flag name between + // them. With a custom longseparator that overlaps + // the prefix (e.g. LongSeparator("-") under the + // default "--" prefix), an attacker-controlled + // completion word like "--x" puts the separator + // inside the prefix, making `arg` shorter than + // longprefix. arg.substr(longprefix.size()) would + // then throw std::out_of_range, which escapes the + // parser as a non-args exception (bypassing the + // documented catch(args::Error) idiom) and is + // thrown even under ARGS_NOEXCEPT. + if (separator != chunk.npos && separator >= longprefix.size()) { std::string arg(chunk, 0, separator); if (auto flag = this->Match(arg.substr(longprefix.size()))) diff --git a/test/completion_separator_prefix_overlap.cxx b/test/completion_separator_prefix_overlap.cxx new file mode 100644 index 0000000..773f305 --- /dev/null +++ b/test/completion_separator_prefix_overlap.cxx @@ -0,0 +1,49 @@ +/* Copyright (c) Taylor Richberger + * This code is released under the license described in the LICENSE file + * + * Regression test for an out-of-bounds std::string::substr in the bash + * completion handler. + * + * When a custom LongSeparator overlaps the long prefix (here "-" under the + * default "--" prefix), an attacker-controlled completion word such as + * "--x" makes chunk.find(longseparator) land *inside* the prefix. The + * handler then built `arg` (= chunk up to the separator) shorter than + * longprefix and called arg.substr(longprefix.size()), which threw + * std::out_of_range. + * + * That std::out_of_range is not an args::Error, so it escaped past the + * documented `catch (args::Completion&) / catch (args::Error&)` idiom and + * terminated the program. After the fix the joined-value completion is + * simply skipped when the separator falls inside the prefix, so the normal + * args::Completion path runs instead. + */ + +#include "test_common.hxx" + +#include + +#include "test_helpers.hxx" + +int main() +{ + args::ArgumentParser p("parser"); + args::CompletionFlag c(p, {"completion"}); + args::ValueFlag f(p, "f", "f", {'f', "foo"}, "abc"); + p.LongSeparator("-"); // legal, non-empty separator that overlaps "--" + + // Must throw args::Completion (the normal completion control-flow), not + // a leaked std::out_of_range. + test::require_throws_as([&] + { + p.ParseArgs(std::vector{"--completion", "bash", "1", "test", "--x"}); + }); + + // A separator exactly at the prefix boundary leaves an empty flag name; + // it must also complete cleanly rather than throw out_of_range. + test::require_throws_as([&] + { + p.ParseArgs(std::vector{"--completion", "bash", "1", "test", "---x"}); + }); + + return 0; +} diff --git a/test/noexcept_completion_separator_prefix_overlap.cxx b/test/noexcept_completion_separator_prefix_overlap.cxx new file mode 100644 index 0000000..bf09b6e --- /dev/null +++ b/test/noexcept_completion_separator_prefix_overlap.cxx @@ -0,0 +1,43 @@ +/* Copyright (c) Taylor Richberger + * This code is released under the license described in the LICENSE file + * + * ARGS_NOEXCEPT counterpart of completion_separator_prefix_overlap. + * + * std::string::substr throws std::out_of_range regardless of ARGS_NOEXCEPT, + * so the unguarded arg.substr(longprefix.size()) in the bash completion + * handler broke the no-throw contract: an ARGS_NOEXCEPT build (which may be + * compiled with -fno-exceptions) would std::terminate on the completion + * word "--x" when a custom LongSeparator overlaps the long prefix. + * + * After the fix ParseArgs returns normally and records the completion via + * the error state instead of throwing. + */ + +#define ARGS_NOEXCEPT +#include "test_common.hxx" + +#include + +#include "test_helpers.hxx" + +int main() +{ + args::ArgumentParser p("parser"); + args::CompletionFlag c(p, {"completion"}); + args::ValueFlag f(p, "f", "f", {'f', "foo"}, "abc"); + p.LongSeparator("-"); + + test::require_nothrow([&] + { + p.ParseArgs(std::vector{"--completion", "bash", "1", "test", "--x"}); + }); + test::require(p.GetError() == args::Error::Completion); + + test::require_nothrow([&] + { + p.ParseArgs(std::vector{"--completion", "bash", "1", "test", "---x"}); + }); + test::require(p.GetError() == args::Error::Completion); + + return 0; +}