diff --git a/src/builtin_string.cpp b/src/builtin_string.cpp index 4e15b8aec..7f1722eab 100644 --- a/src/builtin_string.cpp +++ b/src/builtin_string.cpp @@ -724,17 +724,19 @@ static int string_length(parser_t &parser, io_streams_t &streams, int argc, cons class string_matcher_t { protected: - options_t opts; + const options_t opts; io_streams_t &streams; - int total_matched; + int total_matched{0}; public: string_matcher_t(options_t opts_, io_streams_t &streams_) - : opts(std::move(opts_)), streams(streams_), total_matched(0) {} + : opts(std::move(opts_)), streams(streams_) {} virtual ~string_matcher_t() = default; virtual bool report_matches(const wcstring &arg) = 0; int match_count() const { return total_matched; } + + virtual bool is_valid() const = 0; }; class wildcard_matcher_t final : public string_matcher_t { @@ -785,6 +787,8 @@ class wildcard_matcher_t final : public string_matcher_t { } return true; } + + bool is_valid() const override { return true; } }; static wcstring pcre2_strerror(int err_code) { @@ -795,12 +799,11 @@ static wcstring pcre2_strerror(int err_code) { } struct compiled_regex_t { - pcre2_code *code; - pcre2_match_data *match; + pcre2_code *code{nullptr}; + pcre2_match_data *match{nullptr}; compiled_regex_t(const wchar_t *argv0, const wcstring &pattern, bool ignore_case, - io_streams_t &streams) - : code(nullptr), match(nullptr) { + io_streams_t &streams) { // Disable some sequences that can lead to security problems. uint32_t options = PCRE2_NEVER_UTF; #if PCRE2_CODE_UNIT_WIDTH < 32 @@ -829,6 +832,11 @@ struct compiled_regex_t { pcre2_match_data_free(match); pcre2_code_free(code); } + + bool is_valid() const { return this->code != nullptr; } + + compiled_regex_t(const compiled_regex_t &) = delete; + void operator=(const compiled_regex_t &) = delete; }; class pcre2_matcher_t final : public string_matcher_t { @@ -1032,10 +1040,7 @@ class pcre2_matcher_t final : public string_matcher_t { bool report_matches(const wcstring &arg) override { // A return value of true means all is well (even if no matches were found), false indicates // an unrecoverable error. - if (regex.code == nullptr) { - // pcre2_compile() failed. - return false; - } + assert(regex.code && "report_matches should only be called if the regex was valid"); regex_importer_t var_importer(this->parser, arg, this->regex, opts.all); @@ -1046,7 +1051,6 @@ class pcre2_matcher_t final : public string_matcher_t { // init() directly reports errors itself so it can specify the problem variable return false; } - // See pcre2demo.c for an explanation of this logic. PCRE2_SIZE arglen = arg.length(); auto rc = report_match(arg, pcre2_match(regex.code, PCRE2_SPTR(arg.c_str()), arglen, 0, 0, @@ -1101,6 +1105,8 @@ class pcre2_matcher_t final : public string_matcher_t { imported_vars |= had_match; return true; } + + bool is_valid() const override { return regex.is_valid(); } }; static int string_match(parser_t &parser, io_streams_t &streams, int argc, const wchar_t **argv) { @@ -1131,6 +1137,10 @@ static int string_match(parser_t &parser, io_streams_t &streams, int argc, const } else { matcher = make_unique(cmd, pattern, opts, streams); } + if (!matcher->is_valid()) { + // An error will have been printed by the constructor. + return STATUS_INVALID_ARGS; + } arg_iterator_t aiter(argv, optind, streams); while (const wcstring *arg = aiter.nextstr()) { diff --git a/tests/checks/regex-import.fish b/tests/checks/regex-import.fish index 527951fa4..9347d6744 100644 --- a/tests/checks/regex-import.fish +++ b/tests/checks/regex-import.fish @@ -1,6 +1,10 @@ #RUN: %fish %s # Tests for importing named regex groups as fish variables +# Invalid variable name? +string match --regex -q '(?.*)' -- derp +# CHECKERR: Modification of read-only variable "FISH_VERSION" is not allowed + # Capture first match echo "hello world" | string match --regex -q -- '(?[^ ]+) ?' printf "%s\n" $words