From bb9c5ef32f8e8719763678db022064f5f717b1c2 Mon Sep 17 00:00:00 2001
From: Anoop Saldanha <poonaatsoc@gmail.com>
Date: Tue, 22 Jun 2010 14:10:22 +0530
Subject: [PATCH] in case of duplicate signatures, use the one with the latest revision.  we check for duplicate signatures using the sid and the signature msg

---
 src/detect-engine.c |    2 +
 src/detect-parse.c  |  304 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/detect-parse.h  |    3 +
 src/detect.c        |   11 ++-
 src/detect.h        |   13 ++
 5 files changed, 324 insertions(+), 9 deletions(-)

diff --git a/src/detect-engine.c b/src/detect-engine.c
index c954727..e983853 100644
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -77,6 +77,7 @@ DetectEngineCtx *DetectEngineCtxInit(void) {
     DetectPortDpHashInit(de_ctx);
     ThresholdHashInit(de_ctx);
     VariableNameInitHash(de_ctx);
+    DetectParseDupSigHashInit(de_ctx);
 
     de_ctx->mpm_pattern_id_store = MpmPatternIdTableInitHash();
     if (de_ctx->mpm_pattern_id_store == NULL) {
@@ -104,6 +105,7 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx) {
     SigGroupHeadMpmUriHashFree(de_ctx);
     SigGroupHeadSPortHashFree(de_ctx);
     SigGroupHeadDPortHashFree(de_ctx);
+    DetectParseDupSigHashFree(de_ctx);
     SCSigSignatureOrderingModuleCleanup(de_ctx);
     DetectPortSpHashFree(de_ctx);
     DetectPortDpHashFree(de_ctx);
diff --git a/src/detect-parse.c b/src/detect-parse.c
index a3c96c7..3481a87 100644
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -1305,9 +1305,122 @@ error:
 }
 
 /**
+ * \brief The hash free function to be the used by the hash table -
+ *        DetectEngineCtx->dup_sig_hash_table.
+ *
+ * \param data    Pointer to the data, in our case SigWrapper to be freed.
+ */
+void DetectParseDupSigFreeFunc(void *data)
+{
+    if (data != NULL)
+        free(data);
+
+    return;
+}
+
+/**
+ * \brief The hash function to be the used by the hash table -
+ *        DetectEngineCtx->dup_sig_hash_table.
+ *
+ * \param ht      Pointer to the hash table.
+ * \param data    Pointer to the data, in our case SigWrapper.
+ * \param datalen Not used in our case.
+ *
+ * \retval sw->s->id The generated hash value.
+ */
+uint32_t DetectParseDupSigHashFunc(HashListTable *ht, void *data, uint16_t datalen)
+{
+    SigWrapper *sw = (SigWrapper *)data;
+
+    return (sw->s->id % ht->array_size);
+}
+
+/**
+ * \brief The Compare function to be used by the  hash table -
+ *        DetectEngineCtx->dup_sig_hash_table.
+ *
+ * \param data1 Pointer to the first SigWrapper.
+ * \param len1  Not used.
+ * \param data2 Pointer to the second SigWrapper.
+ * \param len2  Not used.
+ *
+ * \retval 1 If the 2 SigWrappers sent as args match.
+ * \retval 0 If the 2 SigWrappers sent as args do not match.
+ */
+char DetectParseDupSigCompareFunc(void *data1, uint16_t len1, void *data2,
+                                  uint16_t len2)
+{
+    SigWrapper *sw1 = (SigWrapper *)data1;
+    SigWrapper *sw2 = (SigWrapper *)data2;
+
+    if (sw1 == NULL || sw2 == NULL)
+        return 0;
+
+    if (sw1->s->id != sw2->s->id)
+        return 0;
+
+    /* be careful all you non-related signatures with the same sid and no msg.
+     * We treat you all as the same signature */
+    if ((sw1->s->msg == NULL) && (sw2->s->msg == NULL))
+        return 1;
+
+    if ((sw1->s->msg == NULL) || (sw2->s->msg == NULL))
+        return 0;
+
+    if (strlen(sw1->s->msg) != strlen(sw2->s->msg))
+        return 0;
+
+    if (strcmp(sw1->s->msg, sw2->s->msg) != 0)
+        return 0;
+
+    return 1;
+}
+
+/**
+ * \brief Initializes the hash table that is used to cull duplicate sigs.
+ *
+ * \param de_ctx Pointer to the detection engine context.
+ *
+ * \retval  0 On success.
+ * \retval -1 On failure.
+ */
+int DetectParseDupSigHashInit(DetectEngineCtx *de_ctx)
+{
+    de_ctx->dup_sig_hash_table = HashListTableInit(15000,
+                                                   DetectParseDupSigHashFunc,
+                                                   DetectParseDupSigCompareFunc,
+                                                   DetectParseDupSigFreeFunc);
+    if (de_ctx->dup_sig_hash_table == NULL)
+        goto error;
+
+    return 0;
+
+error:
+    return -1;
+}
+
+/**
+ * \brief Frees the hash table that is used to cull duplicate sigs.
+ *
+ * \param de_ctx Pointer to the detection engine context that holds this table.
+ */
+void DetectParseDupSigHashFree(DetectEngineCtx *de_ctx)
+{
+    if (de_ctx->dup_sig_hash_table != NULL)
+        free(de_ctx->dup_sig_hash_table);
+
+    de_ctx->dup_sig_hash_table = NULL;
+
+    return;
+}
+
+/**
  * \brief Parse and append a Signature into the Detection Engine Context
  *        signature list. If the signature is bidirectional it should append
- *        two Signatures (with the addresses switched).
+ *        two Signatures (with the addresses switched).  Also handle duplicate
+ *        signatures.  In case of duplicate sigs, use the ones that have the
+ *        latest revision.  We use the sid and the msg to identifiy duplicate
+ *        sigs.  If 2 sigs have the same sid and msg, they are duplicates.
  *
  * \param de_ctx Pointer to the Detection Engine Context
  * \param sigstr Pointer to a character string containing the signature to be
@@ -1317,33 +1430,121 @@ error:
  *               on success; NULL on failure
  */
 Signature *DetectEngineAppendSig(DetectEngineCtx *de_ctx, char *sigstr) {
-    Signature *sig = SigInitReal(de_ctx, sigstr);
+    SigWrapper *sw_dup = NULL;
+    SigWrapper *sw = NULL;
+
+    Signature *sig = NULL;
+
+    /* parse the signature */
+    sig = SigInitReal(de_ctx, sigstr);
     if (sig == NULL)
         return NULL;
 
+    /* used for making a duplicate_sig_hash_table entry */
+    sw = SCMalloc(sizeof(SigWrapper));
+    if (sw == NULL) {
+        exit(EXIT_FAILURE);
+    }
+    memset(sw, 0, sizeof(SigWrapper));
+    sw->s = sig;
+
+    /* check if we have a duplicate entry for this signature */
+    sw_dup = HashListTableLookup(de_ctx->dup_sig_hash_table, (void *)sw, 0);
+    /* we don't have a duplicate entry for this sig */
+    if (sw_dup == NULL) {
+        /* add it to the hash table */
+        HashListTableAdd(de_ctx->dup_sig_hash_table, (void *)sw, 0);
+
+        if (de_ctx->sig_list != NULL) {
+            SigWrapper *sw_old = NULL;
+            /* add the s_prev entry for the previously loaded sw in the hash_table */
+            SigWrapper sw_tmp;
+            memset(&sw_tmp, 0, sizeof(SigWrapper));
+
+            /* the topmost sig would be the last loaded sig */
+            sw_tmp.s = de_ctx->sig_list;
+            sw_old = HashListTableLookup(de_ctx->dup_sig_hash_table,
+                                         (void *)&sw_tmp, 0);
+            /* sw_old == NULL case is impossible */
+            sw_old->s_prev = sig;
+        }
+
+        /* add it to the detection engine context's signature list */
+        if (sig->flags & SIG_FLAG_BIDIREC) {
+            if (sig->next != NULL) {
+                sig->next->next = de_ctx->sig_list;
+            } else {
+                goto error;
+            }
+        } else {
+            /* if this sig is the first one, sig_list should be null */
+            sig->next = de_ctx->sig_list;
+        }
+
+        de_ctx->sig_list = sig;
+
+        return sig;
+    }
+
+    /* if we have reached here we have a duplicate entry for this signature.
+     * Check the signature revision.  Store the signature with the latest rev
+     * and discard the other one */
+    if (sw->s->rev <= sw_dup->s->rev) {
+        free(sw);
+        goto error;
+    }
+
+    /* the new sig is of a newer revision than the one that is already in the
+     * list.  Remove the old sig from the list */
+    if (sw_dup->s_prev == NULL) {
+        if (sw_dup->s->flags & SIG_FLAG_BIDIREC) {
+            de_ctx->sig_list = sw_dup->s->next->next;
+            SigFree(sw_dup->s->next);
+        } else {
+            de_ctx->sig_list = sw_dup->s->next;
+        }
+        /* i know we can have the SigFree here and the in else block outside.
+         * That's okay.  The extra line provides clarity */
+        SigFree(sw_dup->s);
+    } else {
+        if (sw_dup->s->flags & SIG_FLAG_BIDIREC) {
+            sw_dup->s_prev->next = sw_dup->s->next->next;
+            SigFree(sw_dup->s->next);
+        } else {
+            sw_dup->s_prev->next = sw_dup->s->next;
+        }
+        SigFree(sw_dup->s);
+    }
+
+    /* make changes to the entry to reflect the presence of the new sig */
+    sw_dup->s = sig;
+    sw_dup->s_prev = NULL;
+
     if (sig->flags & SIG_FLAG_BIDIREC) {
         if (sig->next != NULL) {
             sig->next->next = de_ctx->sig_list;
         } else {
+            /* we will never reach here */
             goto error;
         }
-    }
-    else {
+    } else {
         /* if this sig is the first one, sig_list should be null */
         sig->next = de_ctx->sig_list;
     }
 
     de_ctx->sig_list = sig;
 
+    free(sw);
     /**
      * In DetectEngineAppendSig(), the signatures are prepended and we always return the first one
      * so if the signature is bidirectional, the returned sig will point through "next" ptr
      * to the cloned signatures with the switched addresses
      */
-    return sig;
+    return NULL;
 
 error:
-    if ( sig != NULL ) SigFree(sig);
+    if (sig != NULL)
+        SigFree(sig);
     return NULL;
 }
 
@@ -1491,6 +1692,93 @@ end:
     return result;
 }
 
+/**
+ * \test Parsing duplicate sigs.
+ */
+int SigParseTest07(void) {
+    int result = 0;
+
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    if (de_ctx == NULL)
+        goto end;
+
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:1; rev:1;)");
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:1; rev:1;)");
+
+    result = (de_ctx->sig_list != NULL && de_ctx->sig_list->next == NULL);
+
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
+/**
+ * \test Parsing duplicate sigs.
+ */
+int SigParseTest08(void) {
+    int result = 0;
+
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    if (de_ctx == NULL)
+        goto end;
+
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:1; rev:1;)");
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:1; rev:2;)");
+
+    result = (de_ctx->sig_list != NULL && de_ctx->sig_list->next == NULL &&
+              de_ctx->sig_list->rev == 2);
+
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
+/**
+ * \test Parsing duplicate sigs.
+ */
+int SigParseTest09(void) {
+    int result = 1;
+
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    if (de_ctx == NULL)
+        goto end;
+
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:1; rev:1;)");
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:1; rev:2;)");
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:1; rev:6;)");
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:1; rev:4;)");
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:2; rev:2;)");
+    result &= (de_ctx->sig_list != NULL && de_ctx->sig_list->id == 2 &&
+              de_ctx->sig_list->rev == 2);
+    result &= (de_ctx->sig_list->next != NULL && de_ctx->sig_list->next->id == 1 &&
+              de_ctx->sig_list->next->rev == 6);
+    if (result == 0)
+        goto end;
+
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:2; rev:1;)");
+    result &= (de_ctx->sig_list != NULL && de_ctx->sig_list->id == 2 &&
+              de_ctx->sig_list->rev == 2);
+    result &= (de_ctx->sig_list->next != NULL && de_ctx->sig_list->next->id == 1 &&
+              de_ctx->sig_list->next->rev == 6);
+    if (result == 0)
+        goto end;
+
+    DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"boo\"; sid:2; rev:4;)");
+    result &= (de_ctx->sig_list != NULL && de_ctx->sig_list->id == 2 &&
+              de_ctx->sig_list->rev == 4);
+    result &= (de_ctx->sig_list->next != NULL && de_ctx->sig_list->next->id == 1 &&
+              de_ctx->sig_list->next->rev == 6);
+    if (result == 0)
+        goto end;
+
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
 /** \test Direction operator validation (invalid) */
 int SigParseBidirecTest06 (void) {
     int result = 1;
@@ -2342,6 +2630,7 @@ end:
         DetectEngineCtxFree(de_ctx);
     return result;
 }
+
 #endif /* UNITTESTS */
 
 void SigParseRegisterTests(void) {
@@ -2352,6 +2641,9 @@ void SigParseRegisterTests(void) {
     UtRegisterTest("SigParseTest04", SigParseTest04, 1);
     UtRegisterTest("SigParseTest05", SigParseTest05, 1);
     UtRegisterTest("SigParseTest06", SigParseTest06, 1);
+    UtRegisterTest("SigParseTest07", SigParseTest07, 1);
+    UtRegisterTest("SigParseTest08", SigParseTest08, 1);
+    UtRegisterTest("SigParseTest09", SigParseTest09, 1);
 
     UtRegisterTest("SigParseBidirecTest06", SigParseBidirecTest06, 1);
     UtRegisterTest("SigParseBidirecTest07", SigParseBidirecTest07, 1);
diff --git a/src/detect-parse.h b/src/detect-parse.h
index 98027e5..e270ac0 100644
--- a/src/detect-parse.h
+++ b/src/detect-parse.h
@@ -61,5 +61,8 @@ void SigMatchAppendPacket(Signature *, SigMatch *);
 void SigMatchAppendUricontent(Signature *, SigMatch *);
 void SigMatchAppendAppLayer(Signature *, SigMatch *);
 
+int DetectParseDupSigHashInit(DetectEngineCtx *);
+void DetectParseDupSigHashFree(DetectEngineCtx *);
+
 #endif /* __DETECT_PARSE_H__ */
 
diff --git a/src/detect.c b/src/detect.c
index 7af6048..5623a72 100644
--- a/src/detect.c
+++ b/src/detect.c
@@ -296,7 +296,7 @@ int DetectLoadSigFile(DetectEngineCtx *de_ctx, char *sig_file, int *sigs_tot) {
         if (sig != NULL) {
             SCLogDebug("signature %"PRIu32" loaded", sig->id);
             good++;
-	} else {
+        } else {
             SCLogError(SC_ERR_INVALID_SIGNATURE, "Error parsing signature \"%s\" from "
                  "file %s at line %"PRId32"", line, sig_file, lineno - multiline);
             if (de_ctx->failure_fatal == 1) {
@@ -387,7 +387,7 @@ int SigLoadSignatures (DetectEngineCtx *de_ctx, char *sig_file)
     }
 
     if (ret < 0 && de_ctx->failure_fatal) {
-        SCReturnInt(ret);
+        goto end;
     }
 
     SCSigRegisterSignatureOrderingFuncs(de_ctx);
@@ -406,7 +406,12 @@ int SigLoadSignatures (DetectEngineCtx *de_ctx, char *sig_file)
 
     /* Setup the signature group lookup structure and pattern matchers */
     SigGroupBuild(de_ctx);
-    SCReturnInt(0);
+
+    ret = 0;
+
+ end:
+    DetectParseDupSigHashFree(de_ctx);
+    SCReturnInt(ret);
 }
 
 /**
diff --git a/src/detect.h b/src/detect.h
index b0e263b..bd95977 100644
--- a/src/detect.h
+++ b/src/detect.h
@@ -381,6 +381,16 @@ typedef struct ThresholdCtx_    {
     uint32_t th_size;
 } ThresholdCtx;
 
+/**
+ * \brief We use this as data to the hash table DetectEngineCtx->dup_sig_hash_table.
+ */
+typedef struct SigWrapper_ {
+    /* the signature we want to wrap */
+    Signature *s;
+    /* the signature right before the above signatue in the det_ctx->sig_list */
+    Signature *s_prev;
+} SigWrapper;
+
 /** \brief main detection engine ctx */
 typedef struct DetectEngineCtx_ {
     uint8_t flags;
@@ -428,6 +438,9 @@ typedef struct DetectEngineCtx_ {
     HashListTable *variable_names;
     uint16_t variable_names_idx;
 
+    /* hash table used to cull out duplicate sigs */
+    HashListTable *dup_sig_hash_table;
+
     /* memory counters */
     uint32_t mpm_memory_size;
 
-- 
1.7.0.2