Pull request #3904: ips_options: add gadget check for vba_data

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

Squashed commit of the following:

commit c9ec58b0e031465bcd69331fcef82e6dd6f03c5d
Author: Andrii Serbeniuk <aserbeni@cisco.com>
Date:   Mon Jul 10 11:53:46 2023 +0300

    ips_options: update dev_notes about IPS options input values

commit 5f6a0b16d628f524961f56f8ab68b614a39ad390
Author: Andrii Serbeniuk <aserbeni@cisco.com>
Date:   Mon Jul 10 11:18:38 2023 +0300

    ips_options: add unit tests for vba_data

commit f7e319f06441b476463a2b64e786330c0a24425a
Author: Andrii Serbeniuk <aserbeni@cisco.com>
Date:   Mon Jul 10 11:17:45 2023 +0300

    ips_options: add gadget check for vba_data

diff --git a/src/ips_options/dev_notes.txt b/src/ips_options/dev_notes.txt
index b3efb7f99699b5e731588fab36d104b289dcc897..5a85f9bb39ed15a713c464c88c33baf6d227b27d 100644 (file)
--- a/src/ips_options/dev_notes.txt
+++ b/src/ips_options/dev_notes.txt
@@ -6,6 +6,12 @@ however, such as content, are still tightly coupled with the code and can
 only be built statically.  The code will hopefully evolve and eliminate
 these cases.
 
+IPS options are not guaranteed to get all they need for evaluation. For example,
+a packet may not have a flow assigned to it, or the flow may not have a gadget
+present. Not all options need specific inputs, so, if one does, that option should
+validate input values.
+Service-only options may not be guaranteed to get a gadget on the flow.
+
 Several options use RangeCheck to implement upper and/or lower bound
 semantics.  The Snort 2X options had various implementations of ranges so
 3X differs in some places.

diff --git a/src/ips_options/ips_vba_data.cc b/src/ips_options/ips_vba_data.cc
index 73e7ffd760d2da434d8669797f41d9e4feb2aa50..d536168b3ca0b7017b40399b7705a6129b00c9a3 100644 (file)
--- a/src/ips_options/ips_vba_data.cc
+++ b/src/ips_options/ips_vba_data.cc
@@ -39,6 +39,9 @@ IpsOption::EvalStatus VbaDataOption::eval(Cursor& c, Packet* p)
 {
     RuleProfile profile(vbaDataPerfStats);
 
+    if (!p->flow or !p->flow->gadget)
+        return NO_MATCH;
+
     InspectionBuffer buf;
     if (!p->flow->gadget->get_fp_buf(buf.IBT_VBA, p, buf)) 
         return NO_MATCH;
@@ -147,3 +150,37 @@ const BaseApi* ips_vba_data[] =
     nullptr
 };
 
+//-------------------------------------------------------------------------
+// UNIT TESTS
+//-------------------------------------------------------------------------
+#ifdef UNIT_TEST
+
+#include "catch/snort_catch.h"
+
+TEST_CASE("vba_data test", "[ips_vba_data]")
+{
+    VbaDataOption vba_data_opt;
+    Packet p;
+    p.data = (const uint8_t*) "foo";
+    p.dsize = strlen((const char*) p.data);
+
+    SECTION("null flow")
+    {
+        p.flow = nullptr;
+
+        Cursor c(&p);
+        REQUIRE(vba_data_opt.eval(c, &p) == IpsOption::NO_MATCH);
+    }
+
+    SECTION("null gadget")
+    {
+        Flow f;
+        p.flow = &f;
+        p.flow->gadget = nullptr;
+
+        Cursor c(&p);
+        REQUIRE(vba_data_opt.eval(c, &p) == IpsOption::NO_MATCH);
+    }
+}
+
+#endif