From: Oleksii Shumeiko -X (oshumeik - SOFTSERVE INC at Cisco) Date: Tue, 13 Sep 2022 14:23:25 +0000 (+0000) Subject: Pull request #3581: ips_options: content retry X-Git-Tag: 3.1.42.0~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7b613b6d3050a7e855a5ba320925425c77c80ca6;p=thirdparty%2Fsnort3.git Pull request #3581: ips_options: content retry Merge in SNORT/snort3 from ~ASERBENI/snort3:russ_content_retry to master Squashed commit of the following: commit 25963f71586de50ed65369b8823a3bd3e2513d98 Author: Vitalii Date: Tue Sep 13 15:33:25 2022 +0300 trace: ips variables are dumped as hex commit 0efc6a4894c4b65e7b872236b4d8c7bc63e362cd Author: russ Date: Mon Aug 15 21:00:31 2022 -0400 content: fix retry This deprecates the 2nd/"orig" cursor argument to retry. The existing Cursor.delta member provides the required information. The use of byte_extract variables is also fixed for content. Those valuse are used as sizes or offsets and can not be negative. commit f32ed3f56ce9dab991d7951c9c9107fe83137323 Author: russ Date: Tue Aug 16 11:40:40 2022 -0400 ips: trace all node evaluations Iterative evaluations due to retry were not previously traced. --- diff --git a/src/detection/detection_options.cc b/src/detection/detection_options.cc index 488bcb6a6..ab2fb627a 100644 --- a/src/detection/detection_options.cc +++ b/src/detection/detection_options.cc @@ -565,7 +565,7 @@ int detection_option_node_evaluate( rule_vars.reserve(sizeof(var_buf)); for ( unsigned i = 0; i < NUM_IPS_OPTIONS_VARS; ++i ) { - safe_snprintf(var_buf, sizeof(var_buf), "var[%d]=%d ", i, tmp_byte_extract_vars[i]); + safe_snprintf(var_buf, sizeof(var_buf), "var[%u]=0x%X ", i, tmp_byte_extract_vars[i]); rule_vars.append(var_buf); } debug_logf(detection_trace, TRACE_RULE_VARS, p, "Rule options variables: %s\n", @@ -698,8 +698,10 @@ int detection_option_node_evaluate( // We're essentially checking this node again and it potentially // might match again if ( continue_loop ) + { state.checks++; - + node_eval_trace(node, cursor, eval_data.p); + } loop_count++; } while ( continue_loop ); diff --git a/src/detection/pattern_match_data.h b/src/detection/pattern_match_data.h index 8297ea6f0..e0a1feceb 100644 --- a/src/detection/pattern_match_data.h +++ b/src/detection/pattern_match_data.h @@ -57,7 +57,7 @@ struct PatternMatchData unsigned pattern_size; // size of app layer pattern int offset; // pattern search start offset - int depth; // pattern search depth + unsigned depth; // pattern search depth enum { diff --git a/src/framework/ips_option.h b/src/framework/ips_option.h index cbd935bca..9264832e2 100644 --- a/src/framework/ips_option.h +++ b/src/framework/ips_option.h @@ -78,6 +78,8 @@ public: // packet threads virtual bool is_relative() { return false; } + + // 2nd cursor is deprecated, do not use virtual bool retry(Cursor&, const Cursor&) { return false; } virtual void action(Packet*) { } diff --git a/src/ips_options/ips_content.cc b/src/ips_options/ips_content.cc index 1b00b05ca..2083d659f 100644 --- a/src/ips_options/ips_content.cc +++ b/src/ips_options/ips_content.cc @@ -163,7 +163,7 @@ protected: ContentData* config; }; -bool ContentOption::retry(Cursor& current_cursor, const Cursor& orig_cursor) +bool ContentOption::retry(Cursor& c, const Cursor&) { if ( config->pmd.is_negated() ) return false; @@ -171,12 +171,7 @@ bool ContentOption::retry(Cursor& current_cursor, const Cursor& orig_cursor) if ( !config->pmd.depth ) return true; - // 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; + return c.get_delta() + config->pmd.pattern_size <= config->pmd.depth; } uint32_t ContentOption::hash() const @@ -277,14 +272,15 @@ bool ContentOption::operator==(const IpsOption& ips) const */ static int uniSearchReal(ContentData* cd, Cursor& c) { - int offset, depth; + // byte_extract variables are strictly unsigned, used for sizes and forward offsets + // converting from uint32_t to int64_t ensures all extracted values remain positive + int64_t offset, depth; - /* Get byte_extract variables */ if (cd->offset_var >= 0 && cd->offset_var < NUM_IPS_OPTIONS_VARS) { uint32_t extract; GetVarValueByIndex(&extract, cd->offset_var); - offset = (int)extract; + offset = extract; } else offset = cd->pmd.offset; @@ -293,13 +289,12 @@ static int uniSearchReal(ContentData* cd, Cursor& c) { uint32_t extract; GetVarValueByIndex(&extract, cd->depth_var); - depth = (int)extract; + depth = extract; } else depth = cd->pmd.depth; - int pos = c.get_delta(); - int file_pos = c.get_file_pos(); + uint32_t file_pos = c.get_file_pos(); if (file_pos and cd->offset_set) { @@ -307,28 +302,35 @@ static int uniSearchReal(ContentData* cd, Cursor& c) if (offset < 0) return 0; } - if ( !pos ) + + int64_t pos; + + if ( !c.get_delta() ) + // first - adjust from cursor or buffer start + pos = (cd->pmd.is_relative() ? c.get_pos() : 0) + offset; + else { - if ( cd->pmd.is_relative() ) - pos = c.get_pos(); + // retry - adjust from start of last match + pos = c.get_pos() - cd->pmd.pattern_size + cd->match_delta; - pos += offset; + if ( depth ) + depth -= c.get_delta(); } - if ( pos < 0 ) - pos = 0; + if ( pos < 0 or depth < 0 or pos >= c.size() ) + return -1; - int len = c.size() - pos; + int64_t len = c.size() - pos; if ( !depth || len < depth ) depth = len; - unsigned end = pos + cd->pmd.pattern_size; + int64_t end = pos + cd->pmd.pattern_size; // If the pattern size is greater than the amount of data we have to // search, there's no way we can match, but return 0 here for the // case where the match is inverted and there is at least some data. - if ( end > c.size() || (int)end > pos + depth ) + if ( end > c.size() || end > pos + depth ) { if ( cd->pmd.is_negated() && (depth > 0) ) return 0; @@ -336,17 +338,13 @@ 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); + int found = cd->searcher->search(search_handle, base, (unsigned)depth); if ( found >= 0 ) { - int at = pos + found; - c.set_delta(at + cd->match_delta); - c.set_pos(at + cd->pmd.pattern_size); + c.set_delta(c.get_delta() + found + cd->match_delta); + c.set_pos(pos + found + cd->pmd.pattern_size); return 1; } @@ -484,15 +482,7 @@ static void parse_depth(ContentData* cd, const char* data) if (isdigit(data[0]) || data[0] == '-') { - cd->pmd.depth = parse_int(data, "depth"); - - /* check to make sure that this the depth allows this rule to fire */ - if (cd->pmd.depth < (int)cd->pmd.pattern_size) - { - ParseError("the depth (%d) is less than the size of the content(%u)", - cd->pmd.depth, cd->pmd.pattern_size); - return; - } + cd->pmd.depth = parse_int(data, "depth", cd->pmd.pattern_size); cd->depth_var = IPS_OPTIONS_NO_VAR; } else @@ -555,13 +545,7 @@ static void parse_within(ContentData* cd, const char* data) if (isdigit(data[0]) || data[0] == '-') { - cd->pmd.depth = parse_int(data, "within"); - - if (cd->pmd.depth < (int)cd->pmd.pattern_size) - { - ParseError("within (%d) is smaller than size of pattern", cd->pmd.depth); - return; - } + cd->pmd.depth = parse_int(data, "within", cd->pmd.pattern_size); cd->depth_var = IPS_OPTIONS_NO_VAR; } else @@ -574,7 +558,6 @@ static void parse_within(ContentData* cd, const char* data) } } - cd->pmd.set_relative(); } diff --git a/src/parser/parse_utils.cc b/src/parser/parse_utils.cc index 13c0a252f..b06650ff9 100644 --- a/src/parser/parse_utils.cc +++ b/src/parser/parse_utils.cc @@ -144,10 +144,72 @@ int parse_int(const char* data, const char* tag, int low, int high) if ((value > high) || (value < low)) { - ParseError("'%s' must in %d:%d", tag, low, high); + ParseError("'%s' must be in %d:%d, inclusive", tag, low, high); return value; } return value; } +//-------------------------------------------------------------------------- +// unit tests +//-------------------------------------------------------------------------- + +#ifdef UNIT_TEST + +#include "catch/snort_catch.h" + +TEST_CASE("parse_int", "[ParseUtils]") +{ + SECTION("invalid format") + { + int res = parse_int("10 ", "test"); + + CHECK(res == 10); + } + + SECTION("not a number") + { + const char* data = "test"; + + int res = parse_int(data, data); + + CHECK(res == 0); + } + + SECTION("int overflow") + { + const char* data = "99999999999999999999999999999999999999999999999999"; + + int res = parse_int(data, "test"); + + CHECK(res == -1); + CHECK(errno == ERANGE); + } + + SECTION("int underflow") + { + const char* data = "-9999999999999999999999999999999999999999999999999"; + + int res = parse_int(data, "test"); + + CHECK(res == 0); + CHECK(errno == ERANGE); + } + + SECTION("below the limit") + { + int res = parse_int("1", "test", 2, 3); + + CHECK(res == 1); + } + + SECTION("above the limit") + { + int res = parse_int("4", "test", 2, 3); + + CHECK(res == 4); + } +} + +#endif