From: Steve Chew (stechew) Date: Tue, 20 Oct 2020 20:33:48 +0000 (+0000) Subject: Merge pull request #2527 in SNORT/snort3 from ~STECHEW/snort3:ips_infinite_loop to... X-Git-Tag: 3.0.3-3~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f60d667e6bb212fd8e3b3d56dffda0fcad657b63;p=thirdparty%2Fsnort3.git Merge pull request #2527 in SNORT/snort3 from ~STECHEW/snort3:ips_infinite_loop to master Squashed commit of the following: commit acc6832a9d351f2376404f3be7596c29e93993f8 Author: Steve Chew Date: Thu Oct 1 15:45:47 2020 -0400 ips_options: Fix retry calculation in IPS content when handling "within" field. --- diff --git a/src/detection/detection_options.cc b/src/detection/detection_options.cc index b9e311c7f..3ba459478 100644 --- a/src/detection/detection_options.cc +++ b/src/detection/detection_options.cc @@ -355,7 +355,7 @@ int detection_option_node_evaluate( RuleContext profile(state); int result = 0; - int rval = (int)IpsOption::NO_MATCH; // FIXIT-L refactor to eliminate casts to int + int rval; char tmp_noalert_flag = 0; Cursor cursor = orig_cursor; bool continue_loop = true; @@ -411,6 +411,7 @@ int detection_option_node_evaluate( // No, haven't evaluated this one before... Check it. do { + rval = (int)IpsOption::NO_MATCH; // FIXIT-L refactor to eliminate casts to int. if ( node->otn ) { SnortProtocolId snort_protocol_id = p->get_snort_protocol_id(); @@ -681,7 +682,7 @@ int detection_option_node_evaluate( if ( continue_loop && rval == (int)IpsOption::MATCH && node->relative_children ) { IpsOption* opt = (IpsOption*)node->option_data; - continue_loop = opt->retry(cursor); + continue_loop = opt->retry(cursor, orig_cursor); } else continue_loop = false; diff --git a/src/framework/ips_option.h b/src/framework/ips_option.h index afe1539ed..8eb886097 100644 --- a/src/framework/ips_option.h +++ b/src/framework/ips_option.h @@ -87,7 +87,7 @@ public: // packet threads virtual bool is_relative() { return false; } - virtual bool retry(Cursor&) { return false; } + virtual bool retry(Cursor&, const Cursor&) { return false; } virtual void action(Packet*) { } enum EvalStatus { NO_MATCH, MATCH, NO_ALERT, FAILED_BIT }; diff --git a/src/ips_options/ips_content.cc b/src/ips_options/ips_content.cc index 62e6a1884..4d918324f 100644 --- a/src/ips_options/ips_content.cc +++ b/src/ips_options/ips_content.cc @@ -144,7 +144,7 @@ public: bool is_relative() override { return config->pmd.is_relative(); } - bool retry(Cursor&) override; + bool retry(Cursor&, const Cursor&) override; ContentData* get_data() { return config; } @@ -162,7 +162,7 @@ protected: ContentData* config; }; -bool ContentOption::retry(Cursor& c) +bool ContentOption::retry(Cursor& current_cursor, const Cursor& orig_cursor) { if ( config->pmd.is_negated() ) return false; @@ -170,11 +170,10 @@ bool ContentOption::retry(Cursor& c) if ( !config->pmd.depth ) return true; - // FIXIT-L consider moving adjusting delta from eval to retry - assert(c.get_delta() >= config->match_delta); - - unsigned min = c.get_delta() + config->pmd.pattern_size; - unsigned max = c.get_delta() - config->match_delta + config->pmd.offset + config->pmd.depth; + // Do not retry if the current pattern would extend beyond the area + // in which we're looking for it. + unsigned min = current_cursor.get_delta() + config->pmd.pattern_size; + unsigned max = orig_cursor.get_delta() + config->pmd.offset + config->pmd.depth; return min <= max; } @@ -331,6 +330,9 @@ static int uniSearchReal(ContentData* cd, Cursor& c) return -1; } + // FIXIT-M: The depth on retries should not be the same as that used + // on the first try. + const uint8_t* base = c.buffer() + pos; int found = cd->searcher->search(search_handle, base, depth); diff --git a/src/ips_options/ips_pcre.cc b/src/ips_options/ips_pcre.cc index ddced6284..b5812f99a 100644 --- a/src/ips_options/ips_pcre.cc +++ b/src/ips_options/ips_pcre.cc @@ -444,7 +444,7 @@ public: { return (config->options & SNORT_PCRE_RELATIVE) != 0; } EvalStatus eval(Cursor&, Packet*) override; - bool retry(Cursor&) override; + bool retry(Cursor&, const Cursor&) override; PcreData* get_data() { return config; } @@ -593,7 +593,7 @@ IpsOption::EvalStatus PcreOption::eval(Cursor& c, Packet* p) // using content, but more advanced pcre won't work for the relative / // overlap case. -bool PcreOption::retry(Cursor&) +bool PcreOption::retry(Cursor&, const Cursor&) { if ((config->options & (SNORT_PCRE_INVERT | SNORT_PCRE_ANCHORED))) { diff --git a/src/ips_options/ips_regex.cc b/src/ips_options/ips_regex.cc index 4daae4710..0932c92f2 100644 --- a/src/ips_options/ips_regex.cc +++ b/src/ips_options/ips_regex.cc @@ -87,7 +87,7 @@ public: bool is_relative() override { return config.pmd.is_relative(); } - bool retry(Cursor&) override; + bool retry(Cursor&, const Cursor&) override; PatternMatchData* get_pattern(SnortProtocolId, RuleDirection) override { return &config.pmd; } @@ -194,7 +194,7 @@ IpsOption::EvalStatus RegexOption::eval(Cursor& c, Packet*) return NO_MATCH; } -bool RegexOption::retry(Cursor&) +bool RegexOption::retry(Cursor&, const Cursor&) { return !is_relative(); } //------------------------------------------------------------------------- diff --git a/src/ips_options/test/ips_regex_test.cc b/src/ips_options/test/ips_regex_test.cc index a35a1b4f3..0fbac8844 100644 --- a/src/ips_options/test/ips_regex_test.cc +++ b/src/ips_options/test/ips_regex_test.cc @@ -315,7 +315,7 @@ TEST(ips_regex_option, match_absolute) Cursor c(&pkt); CHECK(opt->eval(c, &pkt) == IpsOption::MATCH); CHECK(!strcmp((const char*) c.start(), " stew *")); - CHECK(opt->retry(c)); + CHECK(opt->retry(c,c)); } TEST(ips_regex_option, no_match_delta) @@ -370,7 +370,7 @@ TEST(ips_regex_option_relative, no_match) CHECK(opt->is_relative()); CHECK(opt->eval(c, &pkt) == IpsOption::NO_MATCH); - CHECK(!opt->retry(c)); + CHECK(!opt->retry(c,c)); } //------------------------------------------------------------------------- diff --git a/src/service_inspectors/http_inspect/ips_http.cc b/src/service_inspectors/http_inspect/ips_http.cc index 8815684c7..d62963632 100644 --- a/src/service_inspectors/http_inspect/ips_http.cc +++ b/src/service_inspectors/http_inspect/ips_http.cc @@ -221,11 +221,11 @@ bool HttpIpsOption::operator==(const IpsOption& ips) const buffer_info == hio.buffer_info; } -bool HttpIpsOption::retry(Cursor& c) +bool HttpIpsOption::retry(Cursor& current_cursor, const Cursor&) { if (buffer_info.type == HTTP_BUFFER_PARAM) { - HttpCursorData* cd = (HttpCursorData*)c.get_data(HttpCursorData::id); + HttpCursorData* cd = (HttpCursorData*)current_cursor.get_data(HttpCursorData::id); if (cd) return cd->retry(); diff --git a/src/service_inspectors/http_inspect/ips_http.h b/src/service_inspectors/http_inspect/ips_http.h index f5db9026f..202b0fb86 100644 --- a/src/service_inspectors/http_inspect/ips_http.h +++ b/src/service_inspectors/http_inspect/ips_http.h @@ -102,7 +102,7 @@ public: EvalStatus eval(Cursor&, snort::Packet*) override; uint32_t hash() const override; bool operator==(const snort::IpsOption& ips) const override; - bool retry(Cursor&) override; + bool retry(Cursor&, const Cursor&) override; static IpsOption* opt_ctor(snort::Module* m, OptTreeNode*) { return new HttpIpsOption((HttpCursorModule*)m); } static void opt_dtor(snort::IpsOption* p) { delete p; }