fp FIXITs

diff --git a/ChangeLog b/ChangeLog
index 0d94f0c393700788bab10a102e6e0684046d9d66..bfecaa839ee4d80d96367384f5212198f97693dd 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,7 @@ fixed)
 -- fixed action leak
 -- testing visibility=hidden
 -- more FIXITs
+-- fp related FIXITs
 
 115
 -- remove share.h

diff --git a/src/detection/fpcreate.cc b/src/detection/fpcreate.cc
index 12057aa06b4fa927889c70467440251b27cae6cb..d6f5394ec3dfba9f46c50b93553c60a277feaa94 100644 (file)
--- a/src/detection/fpcreate.cc
+++ b/src/detection/fpcreate.cc
@@ -946,11 +946,12 @@ static PmType get_pm_type(CursorActionType cat)
     return PM_TYPE__MAX;
 }
 
-static PatternMatchData * get_fp_content(OptTreeNode *otn)
+void set_fp_content(OptTreeNode *otn)
 {
     OptFpList *ofl;
     CursorActionType curr_cat = CAT_SET_RAW;
     FpFoo best;
+    PatternMatchData* pmd = nullptr;
 
     for (ofl = otn->opt_func; ofl != NULL; ofl = ofl->next)
     {
@@ -970,8 +971,17 @@ static PatternMatchData * get_fp_content(OptTreeNode *otn)
 
         tmp->pm_type = get_pm_type(curr_cat);
 
-        if (tmp->fp)
-            return tmp;
+        if ( tmp->fp )
+        {
+            if ( pmd )
+                ParseError("only one fast_pattern content per rule allowed");
+
+            else if ( !pmd_can_be_fp(tmp, curr_cat) )
+                ParseError("content ineligible for fast_pattern matcher");
+
+            else
+                pmd = tmp;
+        }
 
         if ( !pmd_can_be_fp(tmp, curr_cat) )
             continue;
@@ -981,7 +991,29 @@ static PatternMatchData * get_fp_content(OptTreeNode *otn)
         if ( curr.is_better(best) )
             best = curr;
     }
-    return best.pmd;
+    if ( !pmd && best.pmd )
+        best.pmd->fp = 1;
+}
+
+static PatternMatchData* get_fp_content(OptTreeNode *otn)
+{
+    OptFpList *ofl;
+
+    for (ofl = otn->opt_func; ofl != NULL; ofl = ofl->next)
+    {
+        if ( !ofl->context )
+            continue;
+
+        if ( ofl->type != RULE_OPTION_TYPE_CONTENT )
+            continue;
+
+        PatternMatchData* pmd = get_pmd(ofl);
+        assert(pmd);
+
+        if ( pmd->fp )
+            return pmd;
+    }
+    return nullptr;
 }
 
 static int fpFinishPortGroupRule(
@@ -1148,8 +1180,8 @@ static int fpAddPortGroupRule(
     if ( !pg || !otn )
         return -1;
 
-    // skip builtin rules
-    if ( !otn->sigInfo.text_rule )  // FIXIT-H must be set for so rules too!
+    // skip builtin rules, continue for text and so rules
+    if ( !otn->sigInfo.text_rule )
         return -1;
 
     /* Rule not enabled */

diff --git a/src/detection/fpcreate.h b/src/detection/fpcreate.h
index fe1c312eeeb8fc9e343bb24863080506989f8493..4b6daf5f9665ac9b5c84ccd3ec47d2a3301b3527 100644 (file)
--- a/src/detection/fpcreate.h
+++ b/src/detection/fpcreate.h
@@ -200,10 +200,12 @@ PORT_GROUP * fpGetServicePortGroupByOrdinal(sopg_table_t *, int, int, int16_t);
 **  Shows the event stats for the created FastPacketDetection
 */
 void fpShowEventStats(SnortConfig*);
-typedef int (*OtnWalkFcn)(int, struct RuleTreeNode *, struct OptTreeNode *);
+typedef int (*OtnWalkFcn)(int, struct RuleTreeNode *, struct OptTreeNode*);
 void fpWalkOtns(int, OtnWalkFcn);
 void fpDynamicDataFree(void *);
 
+void set_fp_content(struct OptTreeNode*);
+
 const char * PatternRawToContent(const char *pattern, int pattern_len);
 
 #endif  /* __FPCREATE_H__ */

diff --git a/src/flow/flow.cc b/src/flow/flow.cc
index abefe75c054864f0e9aa4e0cbe3af0c753ca7970..df9c671bf4d3cc05681b4af0e598f90485fca551 100644 (file)
--- a/src/flow/flow.cc
+++ b/src/flow/flow.cc
@@ -34,13 +34,6 @@
 
 unsigned FlowData:: flow_id = 0;
 
-// FIXIT-H can't inline SO_PUBLIC ctor and dtor in header or we get problems:
-// ld: warning: direct access in FlowData::FlowData(unsigned int,
-// Inspector*) to global weak symbol vtable for FlowData means the weak
-// symbol cannot be overridden at runtime. This was likely caused by
-// different translation units being compiled with different visibility
-// settings.
-
 SO_PUBLIC FlowData::FlowData(unsigned u, Inspector* ph)
 {
     assert(u > 0);

diff --git a/src/ips_options/ips_content.cc b/src/ips_options/ips_content.cc
index 4fbc6677458f1b10d5b90707e5931816bec34d23..62e79fa15e9518e59b7e599bb2cfb5068015fe7d 100644 (file)
--- a/src/ips_options/ips_content.cc
+++ b/src/ips_options/ips_content.cc
@@ -227,28 +227,6 @@ static PatternMatchData* new_pmd()
     return pmd;
 }
 
-// FIXIT-H must ensure that fast_pattern is applied to 
-// a fast_pattern inspection buffer
-static int fast_pattern_count(OptTreeNode *otn, int list_type)
-{
-    OptFpList* fpl = otn ? otn->opt_func : nullptr;
-    int c = 0;
-
-    while ( fpl )
-    {
-        if ( fpl->type == list_type )
-        {
-            ContentOption* opt = (ContentOption*)fpl->context;
-            PatternMatchData* pmd = opt->get_data();
-
-            if ( pmd->fp )
-                c++;
-        }
-        fpl = fpl->next;
-    }
-    return c;
-}
-
 static int32_t parse_int(
     const char* data, const char* tag, int low = -65535, int high = 65535)
 {
@@ -278,17 +256,11 @@ static int32_t parse_int(
     return value;
 }
 
-static void validate_content(
-    PatternMatchData* pmd, OptTreeNode* otn)
+static void finalize_content(PatternMatchData* pmd, OptTreeNode*)
 {
-    if ( fast_pattern_count(otn, RULE_OPTION_TYPE_CONTENT) > 1 )
-    {
-        ParseError("only one content per rule may be used for fast pattern matching.");
-        return;
-    }
-
     if ( pmd->negated )
-        pmd->last_check = (PmdLastCheck*)SnortAlloc(get_instance_max() * sizeof(*pmd->last_check));
+        pmd->last_check = (PmdLastCheck*)SnortAlloc(
+            get_instance_max() * sizeof(*pmd->last_check));
 }
 
 static void make_precomp(PatternMatchData * idx)
@@ -821,17 +793,17 @@ bool ContentModule::set(const char*, Value& v, SnortConfig*)
         pmd->no_case = 1;
 
     else if ( v.is("fast_pattern") )
-        pmd->fp = 1;  // FIXIT-H must ensure current buffer is fp compatible
+        pmd->fp = 1;
 
     else if ( v.is("fast_pattern_offset") )
     {
         pmd->fp_offset = v.get_long();
-        pmd->fp = 1;  // FIXIT-H must ensure current buffer is fp compatible
+        pmd->fp = 1;
     }
     else if ( v.is("fast_pattern_length") )
     {
         pmd->fp_length = v.get_long();
-        pmd->fp = 1;  // FIXIT-H must ensure current buffer is fp compatible
+        pmd->fp = 1;
     }
     else
         return false;
@@ -857,7 +829,7 @@ static IpsOption* content_ctor(Module* p, OptTreeNode * otn)
 {
     ContentModule* m = (ContentModule*)p;
     PatternMatchData* pmd = m->get_data();
-    validate_content(pmd, otn);
+    finalize_content(pmd, otn);
     return new ContentOption(pmd);
 }
 

diff --git a/src/main/modules.cc b/src/main/modules.cc
index 126cb94aca7c70d06651ae25305c676b5d416b89..fc57e3921f4c3d46f45955ec9e1837256d1cf954 100644 (file)
--- a/src/main/modules.cc
+++ b/src/main/modules.cc
@@ -546,8 +546,9 @@ bool ReferencesModule::set(const char*, Value& v, SnortConfig*)
 static const Parameter alerts_params[] =
 {
     { "alert_file", Parameter::PT_STRING, nullptr, nullptr,
-      "set the alert output file name (FIXIT-H delete if not used)" },
+      "set the alert output file name" }, // FIXIT-H delete if not used
 
+    // FIXIT-L move to fast, full, syslog and delete from here
     { "alert_with_interface_name", Parameter::PT_BOOL, nullptr, "false",
       "include interface in alert info (fast, full, or syslog only)" },
 

diff --git a/src/main/snort.cc b/src/main/snort.cc
index ab72ec611e66a6fd01f16b84b4321c88f32ec0f0..3172101a13d13901269fe4bde54b34e52440ac6e 100644 (file)
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -820,7 +820,7 @@ DAQ_Verdict ProcessPacket(
         p->proto_bits = PROTO_BIT__OTHER;
 
 #if 0
-    // FIXIT-H required until decoders are fixed (josh)
+    // FIXIT-J required until decoders are fixed
     else if ( !p->family && (p->proto_bits & PROTO_BIT__IP) )
         p->proto_bits &= ~PROTO_BIT__IP;
 #endif

diff --git a/src/managers/ips_manager.cc b/src/managers/ips_manager.cc
index 07ac61b16bfd05a80facfe0472bac1442c9285f0..e0f3ab9aaf0c22031a490771d33b8b8f260a3230 100644 (file)
--- a/src/managers/ips_manager.cc
+++ b/src/managers/ips_manager.cc
@@ -222,7 +222,7 @@ bool IpsManager::option_set(
     }
 
     if ( !set_arg(current_module, current_params, opt, val, sc) )
-        ParseError("invalid argument %s:%s = %s\n", key, opt, val);
+        ParseError("invalid argument %s:%s = %s", key, opt, val);
 
     if ( current_params->is_positional() )
         ++current_params;
@@ -251,7 +251,7 @@ bool IpsManager::option_end(
 
     if ( mod && !mod->end(key, 0, sc) )
     {
-        ParseError("can't finalize %s\n", key);
+        ParseError("can't finalize %s", key);
         current_keyword = nullptr;
         return false;
     }

diff --git a/src/parser/parse_rule.cc b/src/parser/parse_rule.cc
index 1f0b968d8d2866460eb6beb9ec42bb5db5cd3af7..888de798292a6e9bfc35e1513807c1013029176f 100644 (file)
--- a/src/parser/parse_rule.cc
+++ b/src/parser/parse_rule.cc
@@ -1481,8 +1481,10 @@ OptTreeNode* parse_rule_open(SnortConfig* sc, RuleTreeNode& rtn, bool stub)
     OptTreeNode* otn = (OptTreeNode *)SnortAlloc(sizeof(OptTreeNode));
     otn->state = (OtnState*)SnortAlloc(sizeof(OtnState)*get_instance_max());
 
+    if ( !stub )
+        otn->sigInfo.generator = GENERATOR_SNORT_ENGINE;
+
     otn->chain_node_number = otn_count;
-    otn->sigInfo.generator = GENERATOR_SNORT_ENGINE;
     otn->proto = rtn.proto;
     otn->enabled = ScDefaultRuleState();
 
@@ -1516,13 +1518,15 @@ const char* parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* ot
             ParseError("SO rule %s not loaded.", otn->soid);
         else
         {
-            // FIXIT-L why isn't this set already? (don't hardcode)
+            // FIXIT-L gid may be overwritten here
             otn->sigInfo.generator = GENERATOR_SNORT_SHARED;
             entered = true;
             return so_opts;
         }
     }
     
+    set_fp_content(otn);
+
     /* The IPs in the test node get free'd in ProcessHeadNode if there is
      * already a matching RTN.  The portobjects will get free'd when the
      * port var table is free'd */
@@ -1549,7 +1553,8 @@ const char* parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* ot
 
     // FIXIT-L need more reliable way of knowing type of rule instead of hard
     // coding these gids
-    if ( otn->sigInfo.generator == 1 )
+    // do GIDs actually matter anymore (w/o conflict with builtins)?
+    if ( otn->sigInfo.generator == GENERATOR_SNORT_ENGINE )
     {
         otn->sigInfo.text_rule = true;
         detect_rule_count++;
@@ -1561,6 +1566,9 @@ const char* parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* ot
     }
     else
     {
+        if ( !otn->sigInfo.generator )
+            ParseError("gid must set in builtin rules");
+
         if ( otn->num_detection_opts )
             ParseError("builtin rules do not support detection options");