Pull request #3468: ips_options: improve code coverage

Merge in SNORT/snort3 from ~ASERBENI/snort3:ips_options_cov to master

Squashed commit of the following:

commit e560ef95fb156dc6ddfdf8844f3a50fbbc5f4fa7
Author: Andrii Serbeniuk <aserbeni@cisco.com>
Date:   Thu Jun 9 11:24:44 2022 +0300

    ips_options: improve ips_hash and ips_cvs code coverage

diff --git a/src/ips_options/ips_cvs.cc b/src/ips_options/ips_cvs.cc
index efed000563e3592a4373d39e8ee0655c51d01354..31f6cac95b7fcd9403b653aa910c4e2825c46627 100644 (file)
--- a/src/ips_options/ips_cvs.cc
+++ b/src/ips_options/ips_cvs.cc
@@ -456,3 +456,87 @@ const BaseApi* ips_cvs[] =
     nullptr
 };
 
+//-------------------------------------------------------------------------
+// UNIT TESTS
+//-------------------------------------------------------------------------
+#ifdef UNIT_TEST
+
+#include "catch/snort_catch.h"
+
+class StubIpsOption : public IpsOption
+{
+public:
+    StubIpsOption(const char* name, option_type_t option_type) :
+        IpsOption(name, option_type)
+    {}
+};
+
+TEST_CASE("CvsOption test", "[ips_cvs]")
+{
+    SECTION("operator ==")
+    {
+        CvsRuleOption cvs_rule_opt;
+        CvsOption cvs_opt(cvs_rule_opt);
+
+        SECTION("not equal as IpsOptions")
+        {
+            StubIpsOption opt_other("not_cvs",
+                option_type_t::RULE_OPTION_TYPE_OTHER);
+            REQUIRE_FALSE(cvs_opt == opt_other);
+        }
+
+        SECTION("equal types")
+        {
+            CvsOption cvs_opt_other(cvs_rule_opt);
+            REQUIRE(cvs_opt == cvs_opt_other);
+        }
+
+        SECTION("not equal")
+        {
+            CvsRuleOption cvs_rule_opt_other { CvsTypes::CVS_END_OF_ENUM };
+            CvsOption cvs_opt_other(cvs_rule_opt_other);
+            REQUIRE_FALSE(cvs_opt == cvs_opt_other);
+        }
+    }
+
+    SECTION("CvsGetCommand")
+    {
+        SECTION("cmd is nullptr")
+        {
+            CvsCommand* cmd = nullptr;
+            CvsGetCommand(nullptr, nullptr, cmd);
+            REQUIRE(nullptr == cmd);
+        }
+
+        SECTION("line is nullptr")
+        {
+            CvsCommand cmd;
+            CvsGetCommand(nullptr, nullptr, &cmd);
+
+            CHECK(nullptr == cmd.cmd_str);
+            CHECK(0 == cmd.cmd_str_len);
+            CHECK(nullptr == cmd.cmd_arg);
+            REQUIRE(0 == cmd.cmd_arg_len);
+        }
+    }
+
+    SECTION("CvsValidateEntry")
+    {
+        SECTION("null input")
+        {
+            REQUIRE(CVS_ENTRY_VALID == CvsValidateEntry(nullptr, nullptr));
+        }
+    }
+
+    SECTION("CvsDecode")
+    {
+        SECTION("rule option type is unknown")
+        {
+            CvsRuleOption cvs_rule_opt { CvsTypes::CVS_END_OF_ENUM };
+            REQUIRE(CVS_NO_ALERT == CvsDecode((const uint8_t*)"data", 4,
+                &cvs_rule_opt));
+        }
+    }
+}
+
+#endif

diff --git a/src/ips_options/ips_hash.cc b/src/ips_options/ips_hash.cc
index 4cb6fdfa274c74a82160d4945b0e371ecbec0115..5870e85ecef03507644532f777ef2f2c6110c1b0 100644 (file)
--- a/src/ips_options/ips_hash.cc
+++ b/src/ips_options/ips_hash.cc
@@ -143,19 +143,19 @@ bool HashOption::operator==(const IpsOption& ips) const
 
 int HashOption::match(Cursor& c)
 {
-    int offset;
+    unsigned offset;
 
     /* Get byte_extract variables */
     if (config->offset_var >= 0 && config->offset_var < NUM_IPS_OPTIONS_VARS)
     {
         uint32_t extract;
         GetVarValueByIndex(&extract, config->offset_var);
-        offset = (int)extract;
+        offset = extract;
     }
     else
         offset = config->offset;
 
-    int pos = c.get_delta();
+    unsigned pos = c.get_delta();
 
     if ( !pos )
     {
@@ -165,9 +165,6 @@ int HashOption::match(Cursor& c)
         pos += offset;
     }
 
-    if ( pos < 0 )
-        pos = 0;
-
     // 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.
@@ -505,3 +502,139 @@ const BaseApi* ips_sha256 = &sha256_api.base;
 const BaseApi* ips_sha512 = &sha512_api.base;
 //#endif
 
+//-------------------------------------------------------------------------
+// UNIT TESTS
+//-------------------------------------------------------------------------
+#ifdef UNIT_TEST
+
+#include "catch/snort_catch.h"
+
+#define NO_MATCH snort::IpsOption::EvalStatus::NO_MATCH
+#define MATCH snort::IpsOption::EvalStatus::MATCH
+
+TEST_CASE("HashOption test", "[ips_hash]")
+{
+    SECTION("operator ==")
+    {
+        HashMatchData* hmd = new HashMatchData();
+        HashOption hash_opt("sha256", HPI_SHA256, hmd, sha256, SHA256_HASH_SIZE);
+
+        SECTION("not equal as IpsOptions")
+        {
+            HashMatchData* hmd_other = new HashMatchData();
+            HashOption hash_other("not_sha256", HPI_SHA256, hmd_other, sha256, SHA256_HASH_SIZE);
+            REQUIRE_FALSE(hash_opt == hash_other);
+        }
+
+        SECTION("equal as HashOptions")
+        {
+            HashMatchData* hmd_other = new HashMatchData();
+            HashOption hash_other("sha256", HPI_SHA256, hmd_other, sha256, SHA256_HASH_SIZE);
+            REQUIRE(hash_opt == hash_other);
+        }
+
+        SECTION("hash is different")
+        {
+            HashMatchData* hmd_other = new HashMatchData();
+            hmd_other->hash = "other";
+            HashOption hash_other("sha256", HPI_SHA256, hmd_other, sha256, SHA256_HASH_SIZE);
+            REQUIRE_FALSE(hash_opt == hash_other);
+        }
+
+        SECTION("length is different")
+        {
+            HashMatchData* hmd_other = new HashMatchData();
+            hmd_other->length = 42;
+            HashOption hash_other("sha256", HPI_SHA256, hmd_other, sha256, SHA256_HASH_SIZE);
+            REQUIRE_FALSE(hash_opt == hash_other);
+        }
+
+        SECTION("offset is different")
+        {
+            HashMatchData* hmd_other = new HashMatchData();
+            hmd_other->offset = 42;
+            HashOption hash_other("sha256", HPI_SHA256, hmd_other, sha256, SHA256_HASH_SIZE);
+            REQUIRE_FALSE(hash_opt == hash_other);
+        }
+
+        SECTION("offset_var is different")
+        {
+            HashMatchData* hmd_other = new HashMatchData();
+            hmd_other->offset_var = 42;
+            HashOption hash_other("sha256", HPI_SHA256, hmd_other, sha256, SHA256_HASH_SIZE);
+            REQUIRE_FALSE(hash_opt == hash_other);
+        }
+
+        SECTION("negated is different")
+        {
+            HashMatchData* hmd_other = new HashMatchData();
+            hmd_other->relative = true;
+            HashOption hash_other("sha256", HPI_SHA256, hmd_other, sha256, SHA256_HASH_SIZE);
+            REQUIRE_FALSE(hash_opt == hash_other);
+        }
+
+        SECTION("relative is different")
+        {
+            HashMatchData* hmd_other = new HashMatchData();
+            hmd_other->negated = true;
+            HashOption hash_other("sha256", HPI_SHA256, hmd_other, sha256, SHA256_HASH_SIZE);
+            REQUIRE_FALSE(hash_opt == hash_other);
+        }
+    }
+
+    SECTION("HashOption::match")
+    {
+        SECTION("config->offset_var is zero")
+        {
+            HashMatchData* hmd = new HashMatchData();
+            hmd->offset_var = 0;
+            HashOption hash_opt("sha256", HPI_SHA256, hmd, sha256, SHA256_HASH_SIZE);
+            Cursor c;
+            REQUIRE(0 == hash_opt.match(c));
+        }
+
+        SECTION("cursor->delta is not zero")
+        {
+            HashMatchData* hmd = new HashMatchData();
+            HashOption hash_opt("sha256", HPI_SHA256, hmd, sha256, SHA256_HASH_SIZE);
+            Cursor c;
+            c.set_delta(1);
+            REQUIRE(0 == hash_opt.match(c));
+        }
+
+        SECTION("pattern size > data size")
+        {
+            HashMatchData* hmd = new HashMatchData();
+            hmd->length = 10;
+            HashOption hash_opt("sha256", HPI_SHA256, hmd, sha256, SHA256_HASH_SIZE);
+            Cursor c;
+
+            SECTION("config is negated")
+            {
+                hmd->negated = true;
+                REQUIRE(0 == hash_opt.match(c));
+            }
+
+            SECTION("config is not negated")
+            {
+                hmd->negated = false;
+                REQUIRE(-1 == hash_opt.match(c));
+            }
+        }
+    }
+
+    SECTION("HashOption::eval")
+    {
+        SECTION("on match error")
+        {
+            HashMatchData* hmd = new HashMatchData();
+            hmd->length = 10;
+            hmd->negated = false;
+            HashOption hash_opt("sha256", HPI_SHA256, hmd, sha256, SHA256_HASH_SIZE);
+            Cursor c;
+            REQUIRE(NO_MATCH == hash_opt.eval(c, nullptr));
+        }
+    }
+}
+
+#endif