Merge pull request #2509 in SNORT/snort3 from ~MIALTIZE/snort3:wiz_parsing to master

Squashed commit of the following:

commit b7580013b4c9669bc53ca4ab702750844a3716d3
Author: Michael Altizer <mialtize@cisco.com>
Date:   Thu Oct 1 13:59:10 2020 -0400

    wizard: Clean up parameter parsing and make it a bit stricter

    - Fixes Lua implementation-specific ordering dependency of parameter
      parsing for spells and hexes.
    - Adds parse errors for spells and hexes that are missing services or
      patterns.

diff --git a/src/service_inspectors/wizard/wiz_module.cc b/src/service_inspectors/wizard/wiz_module.cc
index c6d2b8ce4a1aca66a8f6f5fab94b5bc151a142e0..dfb621ffa06b51859dc9f346698831ab3bd7f0ce 100644 (file)
--- a/src/service_inspectors/wizard/wiz_module.cc
+++ b/src/service_inspectors/wizard/wiz_module.cc
@@ -153,19 +153,20 @@ bool WizardModule::set(const char*, Value& v, SnortConfig*)
     else if ( v.is("client_first") )
         return true;
 
-    else if ( v.is("hex") )
-        spells.emplace_back(v.get_string());
-
-    else if ( v.is("spell") )
-        spells.emplace_back(v.get_string());
-
+    else if ( v.is("hex") || v.is("spell") )
+    {
+        if (c2s)
+            c2s_patterns.emplace_back(v.get_string());
+        else
+            s2c_patterns.emplace_back(v.get_string());
+    }
     else if ( v.is("curses") )
         curses->add_curse(v.get_string());
 
     return true;
 }
 
-bool WizardModule::begin(const char* fqn, int, SnortConfig*)
+bool WizardModule::begin(const char* fqn, int idx, SnortConfig*)
 {
     if ( !strcmp(fqn, "wizard") )
     {
@@ -179,20 +180,25 @@ bool WizardModule::begin(const char* fqn, int, SnortConfig*)
     }
     else if ( !strcmp(fqn, "wizard.hexes") || !strcmp(fqn, "wizard.spells") )
     {
-        service.clear();
-    }
-    else if ( !strcmp(fqn, "wizard.hexes.to_client") || !strcmp(fqn, "wizard.hexes.to_server") ||
-        !strcmp(fqn, "wizard.spells.to_client") || !strcmp(fqn, "wizard.spells.to_server") )
-    {
-        spells.clear();
+        if ( idx > 0 )
+        {
+            service.clear();
+            c2s_patterns.clear();
+            s2c_patterns.clear();
+        }
     }
+    else if ( !strcmp(fqn, "wizard.hexes.to_client") || !strcmp(fqn, "wizard.spells.to_client") )
+        c2s = false;
+
+    else if ( !strcmp(fqn, "wizard.hexes.to_server") || !strcmp(fqn, "wizard.spells.to_server") )
+        c2s = true;
 
     return true;
 }
 
-bool WizardModule::add_spells(MagicBook* b, string& service, bool hex)
+static bool add_spells(MagicBook* b, const string& service, const vector<string>& patterns, bool hex)
 {
-    for ( const auto& p : spells )
+    for ( const auto& p : patterns )
     {
         const char* val = service.c_str();
         if ( !b->add_spell(p.c_str(), val) )
@@ -218,32 +224,54 @@ bool WizardModule::add_spells(MagicBook* b, string& service, bool hex)
     return true;
 }
 
-bool WizardModule::end(const char* fqn, int, SnortConfig*)
+bool WizardModule::end(const char* fqn, int idx, SnortConfig*)
 {
     if ( !strcmp(fqn, "wizard") )
     {
         service.clear();
-        spells.clear();
-    }
-    else if ( !strcmp(fqn, "wizard.hexes.to_client") )
-    {
-        if ( !add_spells(s2c_hexes, service, true) )
-            return false;
-    }
-    else if ( !strcmp(fqn, "wizard.spells.to_client") )
-    {
-        if ( !add_spells(s2c_spells, service, false) )
-            return false;
+        c2s_patterns.clear();
     }
-    else if ( !strcmp(fqn, "wizard.hexes.to_server") )
+    else if ( !strcmp(fqn, "wizard.hexes") )
     {
-        if ( !add_spells(c2s_hexes, service, true) )
-            return false;
+        if ( idx > 0 )
+        {
+            // Validate the hex
+            if ( service.empty() )
+            {
+                ParseError("Hexes must have a service name");
+                return false;
+            }
+            if ( c2s_patterns.empty() && s2c_patterns.empty() )
+            {
+                ParseError("Hexes must have at least one pattern");
+                return false;
+            }
+            if ( !add_spells(c2s_hexes, service, c2s_patterns, true) )
+                return false;
+            if ( !add_spells(s2c_hexes, service, s2c_patterns, true) )
+                return false;
+        }
     }
-    else if ( !strcmp(fqn, "wizard.spells.to_server") )
+    else if ( !strcmp(fqn, "wizard.spells") )
     {
-        if ( !add_spells(c2s_spells, service, false) )
-            return false;
+        if ( idx > 0 )
+        {
+            // Validate the spell
+            if ( service.empty() )
+            {
+                ParseError("Spells must have a service name");
+                return false;
+            }
+            if ( c2s_patterns.empty() && s2c_patterns.empty() )
+            {
+                ParseError("Spells must have at least one pattern");
+                return false;
+            }
+            if ( !add_spells(c2s_spells, service, c2s_patterns, false) )
+                return false;
+            if ( !add_spells(s2c_spells, service, s2c_patterns, false) )
+                return false;
+        }
     }
 
     return true;

diff --git a/src/service_inspectors/wizard/wiz_module.h b/src/service_inspectors/wizard/wiz_module.h
index 6fe68ff5a8faefa36647c60a89149134cf1034b5..e6d434ea9ebc9c476cd33d29a5aaa12b30ab280a 100644 (file)
--- a/src/service_inspectors/wizard/wiz_module.h
+++ b/src/service_inspectors/wizard/wiz_module.h
@@ -67,12 +67,11 @@ public:
     void set_trace(const snort::Trace*) const override;
     const snort::TraceOption* get_trace_options() const override;
 
-private:
-    bool add_spells(MagicBook*, std::string&, bool hex);
-
 private:
     std::string service;
-    std::vector<std::string> spells;
+    std::vector<std::string> c2s_patterns;
+    std::vector<std::string> s2c_patterns;
+    bool c2s;
 
     MagicBook* c2s_hexes;
     MagicBook* s2c_hexes;