Merge pull request #796 in SNORT/snort3 from sdf-credit-card-fixup to master

Squashed commit of the following:

commit 768f12ad556a273e9b3a6245ef8227c348f5ab16
Author: Victor Roemer <viroemer@cisco.com>
Date:   Wed Feb 1 12:54:01 2017 -0500

    Use assert, simplify test condition in bounds check

commit 5c07187d6b39a494c238474210620ee115ad7c41
Author: Victor Roemer <viroemer@cisco.com>
Date:   Wed Feb 1 11:20:54 2017 -0500

    Fix off by one bounds checking of the right side

commit 656d6d7c0df190ed294b5f185e554d3ee8b37095
Author: Victor Roemer <viroemer@cisco.com>
Date:   Wed Feb 1 10:53:13 2017 -0500

    Add a numeric boundaries test for builtin patterns

commit 7d3cd2287065df80d02102d593752de10095ff9b
Author: Victor Roemer <viroemer@cisco.com>
Date:   Tue Jan 24 14:55:45 2017 -0500

    Re-enable fast pattern and credit card luhn check validation

commit 73b2b2ab23a0949533ad79243f46a8f7fc04e0ed
Author: Victor Roemer <viroemer@cisco.com>
Date:   Tue Jan 24 13:53:05 2017 -0500

    Remove boundry goofiness in Luhn check that broke validation
    Replace digit separator in credit card pattern from `[- ]?` to `\D?`,
    the original pattern works sometimes but not others??

diff --git a/src/ips_options/ips_sd_pattern.cc b/src/ips_options/ips_sd_pattern.cc
index 07e7e3c61e147a739da4b6078dbf6aa7af9574bd..83ebd59ab8f47241650f4afb67890a590536c0a9 100644 (file)
--- a/src/ips_options/ips_sd_pattern.cc
+++ b/src/ips_options/ips_sd_pattern.cc
@@ -24,6 +24,8 @@
 
 #include "ips_sd_pattern.h"
 
+#include <cctype>
+
 #include <hs_compile.h>
 #include <hs_runtime.h>
 
@@ -44,9 +46,9 @@
 #define s_name "sd_pattern"
 #define s_help "rule option for detecting sensitive data"
 
-#define SD_SOCIAL_PATTERN          R"(\b\d{3}-\d{2}-\d{4}\b)"
-#define SD_SOCIAL_NODASHES_PATTERN R"(\b\d{9}\b)"
-#define SD_CREDIT_PATTERN_ALL      R"(\b\d{4}[- ]?\d{4}[- ]?\d{2}[- ]?\d{2}[- ]?\d{3,4}\b)"
+#define SD_SOCIAL_PATTERN          R"(\d{3}-\d{2}-\d{4})"
+#define SD_SOCIAL_NODASHES_PATTERN R"(\d{9})"
+#define SD_CREDIT_PATTERN_ALL      R"(\d{4}\D?\d{4}\D?\d{2}\D?\d{2}\D?\d{3,4})"
 
 // we need to update scratch in the main thread as each pattern is processed
 // and then clone to thread specific after all rules are loaded.  s_scratch is
@@ -82,6 +84,7 @@ struct SdPatternConfig
     std::string pii;
     unsigned threshold = 1;
     bool obfuscate_pii = false;
+    bool forced_boundary = false; 
     int (* validate)(const uint8_t* buf, unsigned long long buflen) = nullptr;
 
     inline bool operator==(const SdPatternConfig& rhs) const
@@ -176,8 +179,31 @@ bool SdPatternOption::operator==(const IpsOption& ips) const
 
 struct hsContext
 {
-    hsContext(const SdPatternConfig& c_, Packet* p_, const uint8_t* const start_)
-        : config(c_), packet(p_), start(start_) { }
+    hsContext(const SdPatternConfig& c_, Packet* p_, const uint8_t* const start_,
+            const uint8_t* _buf, unsigned int _buflen )
+        : config(c_), packet(p_), start(start_), buf(_buf), buflen(_buflen) { }
+
+    bool has_valid_bounds(unsigned long long from, unsigned long long len)
+    {
+        bool left = false;
+        bool right = false;
+
+        // validate the left side
+
+        if ( from == 0 )
+            left = true;
+        else if ( from && !::isdigit((int)buf[from-1]) )
+            left = true;
+
+        // validate the right side
+   
+        if ( from+len == buflen )
+            right = true;
+        else if ( from + len < buflen && !::isdigit((int)buf[from+len]) )
+            right = true;
+
+        return left and right;
+    }
 
     unsigned int count = 0;
 
@@ -185,6 +211,7 @@ struct hsContext
     Packet* packet = nullptr;
     const uint8_t* const start = nullptr;
     const uint8_t* buf = nullptr;
+    unsigned int buflen = 0;
 };
 
 static int hs_match(unsigned int /*id*/, unsigned long long from,
@@ -197,6 +224,10 @@ static int hs_match(unsigned int /*id*/, unsigned long long from,
     assert(ctx->start);
 
     unsigned long long len = to - from;
+
+    if ( ctx->config.forced_boundary && !ctx->has_valid_bounds(from, len) )
+        return 0;
+
     if ( ctx->config.validate && ctx->config.validate(ctx->buf+from, len) != 1 )
         return 0;
 
@@ -224,8 +255,7 @@ unsigned SdPatternOption::SdSearch(Cursor& c, Packet* p)
     SnortState* ss = snort_conf->state + get_instance_id();
     assert(ss->sdpattern_scratch);
 
-    hsContext ctx(config, p, start);
-    ctx.buf = buf;
+    hsContext ctx(config, p, start, buf, buflen);
 
     hs_error_t stat = hs_scan(config.db, (const char*)buf, buflen, 0,
         (hs_scratch_t*)ss->sdpattern_scratch, hs_match, (void*)&ctx);
@@ -318,16 +348,19 @@ bool SdPatternModule::set(const char*, Value& v, SnortConfig* sc)
         config.pii = SD_CREDIT_PATTERN_ALL;
         config.validate = SdLuhnAlgorithm;
         config.obfuscate_pii = sc->obfuscate_pii;
+        config.forced_boundary = true;
     }
     else if (config.pii == "us_social")
     {
         config.pii = SD_SOCIAL_PATTERN;
         config.obfuscate_pii = sc->obfuscate_pii;
+        config.forced_boundary = true;
     }
     else if (config.pii == "us_social_nodashes")
     {
         config.pii = SD_SOCIAL_NODASHES_PATTERN;
         config.obfuscate_pii = sc->obfuscate_pii;
+        config.forced_boundary = true;
     }
 
     return true;

diff --git a/src/ips_options/sd_credit_card.cc b/src/ips_options/sd_credit_card.cc
index d9a5a82b0c8c5ca2778c5b7c335e7592a5d4c7e7..6ce012abc56432a53c5bddff897b06a4158c7fdd 100644 (file)
--- a/src/ips_options/sd_credit_card.cc
+++ b/src/ips_options/sd_credit_card.cc
@@ -26,6 +26,7 @@
 #include "sd_credit_card.h"
 
 #include <cctype>
+#include <cassert>
 
 #define ISSUER_SIZE     4
 #define CC_COPY_BUF_LEN 20 /* 16 digits + 3 spaces/dashes + null */
@@ -70,22 +71,10 @@ int SdLuhnAlgorithm(const uint8_t *buf, unsigned long long buflen)
     char cc_digits[CC_COPY_BUF_LEN]; /* Normalized CC# string */
     uint32_t j;
 
-    if (buf == nullptr || buflen < MIN_CC_BUF_LEN)
-        return 0;
-
-    /* Generally, the buffer has two non-digits, one on either side. Sometimes,
-     * when the buffer is pointing to the first line of the data, it might
-     * start with a digit, instead of a non-digit. Strip the non-digits
-     * only.
-     */
-    if (isdigit((int)buf[0]))
-        buflen -= 1;
+    assert(buf);
 
-    else
-    {
-        buf++;
-        buflen -= 2;
-    }
+    if (buflen < MIN_CC_BUF_LEN)
+        return 0;
 
     /* If the first digit is greater than 6, this isn't one of the major
        credit cards. */