Merge pull request #1640 in SNORT/snort3 from ~MDAGON/snort3:smtp_fix to master

Squashed commit of the following:

commit 5aae8d1c8a125cc53a58efcee29035739a666d7a
Author: mdagon <mdagon@cisco.com>
Date:   Wed Jun 5 11:36:13 2019 -0400

    smtp: pass packet pointer instead of nullptr to SMTP_CopyToAltBuffer

diff --git a/src/mime/file_mime_process.cc b/src/mime/file_mime_process.cc
index e12eebf8df85ffce9fcc143d8ffd92289ad6e0bd..f43e71c05bd7c016481353545420c8484ad21ff7 100644 (file)
--- a/src/mime/file_mime_process.cc
+++ b/src/mime/file_mime_process.cc
@@ -166,7 +166,7 @@ void MimeSession::setup_decode(const char* data, int size, bool cnt_xf)
  *
  * @return  i       index into p->payload where we stopped looking at data
  */
-const uint8_t* MimeSession::process_mime_header(const uint8_t* ptr,
+const uint8_t* MimeSession::process_mime_header(Packet* p, const uint8_t* ptr,
     const uint8_t* data_end_marker)
 {
     const uint8_t* eol = data_end_marker;
@@ -308,7 +308,7 @@ const uint8_t* MimeSession::process_mime_header(const uint8_t* ptr,
             state_flags &= ~MIME_FLAG_DATA_HEADER_CONT;
         }
 
-        int ret = handle_header_line(ptr, eol, max_header_name_len);
+        int ret = handle_header_line(ptr, eol, max_header_name_len, p);
         if (ret < 0)
             return nullptr;
         else if (ret > 0)
@@ -514,7 +514,7 @@ const uint8_t* MimeSession::process_mime_data_paf(
             {
                 /* if we're normalizing and not ignoring data copy data end marker
                  * and dot to alt buffer */
-                if (normalize_data(start, end) < 0)
+                if (normalize_data(start, end, p) < 0)
                     return nullptr;
 
                 reset_mime_state();
@@ -545,12 +545,12 @@ const uint8_t* MimeSession::process_mime_data_paf(
 
     if (data_state == STATE_DATA_HEADER)
     {
-        start = process_mime_header(start, end);
+        start = process_mime_header(p, start, end);
         if (start == nullptr)
             return nullptr;
     }
 
-    if (normalize_data(start, end) < 0)
+    if (normalize_data(start, end, p) < 0)
         return nullptr;
 
     // now we shouldn't have to worry about copying any data to the alt buffer
@@ -561,7 +561,7 @@ const uint8_t* MimeSession::process_mime_data_paf(
         switch (data_state)
         {
         case STATE_MIME_HEADER:
-            start = process_mime_header(start, end);
+            start = process_mime_header(p, start, end);
             break;
         case STATE_DATA_BODY:
             start = process_mime_body(start, end, isFileEnd(position) );

diff --git a/src/mime/file_mime_process.h b/src/mime/file_mime_process.h
index a871672b22e214b178c67a3323c87922d99148ab..549c8151c7c33abd8d7f4e7bcf6aec42cf7bc3b0 100644 (file)
--- a/src/mime/file_mime_process.h
+++ b/src/mime/file_mime_process.h
@@ -86,8 +86,8 @@ private:
     std::string filename;
 
     // SMTP, IMAP, POP might have different implementation for this
-    virtual int handle_header_line(const uint8_t*, const uint8_t*, int) { return 0; }
-    virtual int normalize_data(const uint8_t*, const uint8_t*) { return 0; }
+    virtual int handle_header_line(const uint8_t*, const uint8_t*, int, Packet*) { return 0; }
+    virtual int normalize_data(const uint8_t*, const uint8_t*, Packet*) { return 0; }
     virtual void decode_alert() { }
     virtual void decompress_alert() { }
     virtual void reset_state(Flow*) { }
@@ -95,7 +95,7 @@ private:
 
     void reset_mime_state();
     void setup_decode(const char* data, int size, bool cnt_xf);
-    const uint8_t* process_mime_header(const uint8_t* ptr, const uint8_t* data_end_marker);
+    const uint8_t* process_mime_header(Packet*, const uint8_t* ptr, const uint8_t* data_end_marker);
     const uint8_t* process_mime_body(const uint8_t* ptr, const uint8_t* data_end,bool is_data_end);
     const uint8_t* process_mime_data_paf(Packet*, const uint8_t* start, const uint8_t* end,
         bool upload, FilePosition);

diff --git a/src/service_inspectors/smtp/smtp.cc b/src/service_inspectors/smtp/smtp.cc
index 943d49fc2fd0395f0051e13ce643be7da9ecff48..47fea6ee57374e12619bb66cfaee06682f2a96b6 100644 (file)
--- a/src/service_inspectors/smtp/smtp.cc
+++ b/src/service_inspectors/smtp/smtp.cc
@@ -40,6 +40,10 @@
 #include "smtp_util.h"
 #include "smtp_xlink2state.h"
 
+#ifdef UNIT_TEST
+#include "catch/snort_catch.h"
+#endif
+
 using namespace snort;
 
 THREAD_LOCAL ProfileStats smtpPerfStats;
@@ -180,7 +184,7 @@ SmtpFlowData::SmtpFlowData() : FlowData(inspector_id)
 {
     memset(&session, 0, sizeof(session));
     smtpstats.concurrent_sessions++;
-    if(smtpstats.max_concurrent_sessions < smtpstats.concurrent_sessions)
+    if (smtpstats.max_concurrent_sessions < smtpstats.concurrent_sessions)
         smtpstats.max_concurrent_sessions = smtpstats.concurrent_sessions;
 }
 
@@ -215,7 +219,7 @@ static SMTPData* SetNewSMTPData(SMTP_PROTO_CONF* config, Packet* p)
     smtp_ssn->mime_ssn->config = config;
     smtp_ssn->mime_ssn->set_mime_stats(&(smtpstats.mime_stats));
 
-    if(Stream::is_midstream(p->flow))
+    if (Stream::is_midstream(p->flow))
     {
         smtp_ssn->state = STATE_UNKNOWN;
     }
@@ -367,7 +371,7 @@ static int GetCmdId(SMTP_PROTO_CONF* config, const char* name, SMTPCmdTypeEnum t
     return AddCmd(config, name, type);
 }
 
-static void SMTP_PrintConfig(SMTP_PROTO_CONF *config)
+static void SMTP_PrintConfig(SMTP_PROTO_CONF* config)
 {
     assert(config);
 
@@ -377,13 +381,13 @@ static void SMTP_PrintConfig(SMTP_PROTO_CONF *config)
     LogMessage("SMTP Config:\n");
     snprintf(buf, sizeof(buf) - 1, "    Normalize: ");
 
-    if(config->normalize == NORMALIZE_ALL)
+    if (config->normalize == NORMALIZE_ALL)
         sfsnprintfappend(buf, sizeof(buf) - 1, "all");
 
-    else if(config->normalize == NORMALIZE_NONE)
+    else if (config->normalize == NORMALIZE_NONE)
         sfsnprintfappend(buf, sizeof(buf) - 1, "none");
 
-    else if(config->normalize == NORMALIZE_CMDS)
+    else if (config->normalize == NORMALIZE_CMDS)
     {
         for (SMTPToken* cmd = config->cmds; cmd->name != nullptr; cmd++)
         {
@@ -459,11 +463,11 @@ static void SMTP_PrintConfig(SMTP_PROTO_CONF *config)
         (config->xlink2state == ALERT_XLINK2STATE) ? "Yes" : "No");
     if (config->xlink2state == DROP_XLINK2STATE)
     {
-        LogMessage("    Drop on X-Link2State Alert: %s\n", "Yes" );
+        LogMessage("    Drop on X-Link2State Alert: %s\n", "Yes");
     }
     else
     {
-        LogMessage("    Drop on X-Link2State Alert: %s\n", "No" );
+        LogMessage("    Drop on X-Link2State Alert: %s\n", "No");
     }
 
     snprintf(buf, sizeof(buf) - 1, "    Alert on commands: ");
@@ -984,7 +988,6 @@ static void SMTP_ProcessClientPacket(SMTP_PROTO_CONF* config, Packet* p, SMTPDat
     const uint8_t* ptr = p->data;
     const uint8_t* end = p->data + p->dsize;
 
-
     if (smtp_ssn->state == STATE_CONNECT)
     {
         smtp_ssn->state = STATE_COMMAND;
@@ -1294,7 +1297,7 @@ static void SMTP_RegXtraDataFuncs(SMTP_PROTO_CONF* config)
 }
 
 int SmtpMime::handle_header_line(
-    const uint8_t* ptr, const uint8_t* eol, int max_header_len)
+    const uint8_t* ptr, const uint8_t* eol, int max_header_len, Packet* p)
 {
     /* get length of header line */
     int header_line_len = eol - ptr;
@@ -1306,14 +1309,13 @@ int SmtpMime::handle_header_line(
         (header_line_len > config->max_header_line_len))
     {
         DetectionEngine::queue_event(GID_SMTP, SMTP_DATA_HDR_OVERFLOW);
-
     }
 
     /* Does VRT want data headers normalized?
      * currently the code does not normalize headers */
     if (smtp_normalizing)
     {
-        int ret = SMTP_CopyToAltBuffer(nullptr, ptr, eol - ptr);
+        int ret = SMTP_CopyToAltBuffer(p, ptr, eol - ptr);
 
         if (ret == -1)
             return (-1);
@@ -1330,7 +1332,7 @@ int SmtpMime::handle_header_line(
     return 0;
 }
 
-int SmtpMime::normalize_data(const uint8_t* ptr, const uint8_t* data_end)
+int SmtpMime::normalize_data(const uint8_t* ptr, const uint8_t* data_end, Packet* p)
 {
     /* if we're ignoring data and not already normalizing, copy everything
      * up to here into alt buffer so detection engine doesn't have
@@ -1343,7 +1345,7 @@ int SmtpMime::normalize_data(const uint8_t* ptr, const uint8_t* data_end)
     else */
     if (!config->decode_conf.is_ignore_data() && smtp_normalizing)
     {
-        return SMTP_CopyToAltBuffer(nullptr, ptr, data_end - ptr);
+        return SMTP_CopyToAltBuffer(p, ptr, data_end - ptr);
     }
 
     return 0;
@@ -1378,7 +1380,6 @@ void SmtpMime::reset_state(Flow* ssn)
     SMTP_ResetState(ssn);
 }
 
-
 bool SmtpMime::is_end_of_data(Flow* session)
 {
     return smtp_is_data_end(session);
@@ -1583,3 +1584,54 @@ SO_PUBLIC const BaseApi* snort_plugins[] =
 const BaseApi* sin_smtp = &smtp_api.base;
 #endif
 
+#ifdef UNIT_TEST
+TEST_CASE("handle_header_line", "[smtp]")
+{
+    // Setup
+    MailLogConfig log_config;
+    snort::DecodeConfig decode_conf;
+    log_config.log_email_hdrs = 0;
+    SmtpMime mime_ssn(&decode_conf, &log_config);
+    smtp_normalizing = true;
+    SMTP_PROTO_CONF config;
+    mime_ssn.config = &config;
+    uint8_t ptr[68] = "Date: Tue, 1 Mar 2016 22:37:56 -0500\r\nFrom: acc2 <acc2@localhost>\r\n";
+    uint8_t* eol = ptr + 38;
+    Packet p;
+    p.context = new IpsContext(1);
+    
+    int res = mime_ssn.handle_header_line(ptr, eol, 0, &p);
+    REQUIRE((res == 0));
+    unsigned len = 0;
+    const uint8_t* header = SMTP_GetAltBuffer(&p, len);
+    REQUIRE((len == 38));
+    REQUIRE((memcmp(header, ptr, len)== 0));
+
+    // Cleanup
+    delete p.context;
+}
+
+TEST_CASE("normalize_data", "[smtp]")
+{
+    // Setup
+    MailLogConfig log_config;
+    snort::DecodeConfig decode_conf;
+    SmtpMime mime_ssn(&decode_conf, &log_config);
+    smtp_normalizing = true;
+    uint8_t ptr[23] = "\r\n--wac7ysb48OaltWcw\r\n";
+    uint8_t* data_end = ptr + 22;
+    Packet p;
+    p.context = new IpsContext(1);
+    
+    int res = mime_ssn.normalize_data(ptr, data_end, &p);
+    REQUIRE((res == 0));
+    unsigned len = 0;
+    const uint8_t* data = SMTP_GetAltBuffer(&p, len);
+    REQUIRE((len == 22));
+    REQUIRE((memcmp(data, ptr, len)== 0));
+
+    // Cleanup
+    delete p.context;
+}
+#endif
+

diff --git a/src/service_inspectors/smtp/smtp.h b/src/service_inspectors/smtp/smtp.h
index 031b65fc85d1d5cf5bc5f00cceb0cec112a44aac..6cb9289934c384a2c4325d9a2bedd773e60e78b5 100644 (file)
--- a/src/service_inspectors/smtp/smtp.h
+++ b/src/service_inspectors/smtp/smtp.h
@@ -143,10 +143,15 @@ class SmtpMime : public snort::MimeSession
 public:
     using snort::MimeSession::MimeSession;
     SMTP_PROTO_CONF* config;
+#ifndef UNIT_TEST  
 private:
+#endif  
     int handle_header_line(const uint8_t* ptr, const uint8_t* eol,
-        int max_header_len) override;
-    int normalize_data(const uint8_t* ptr, const uint8_t* data_end) override;
+        int max_header_len, snort::Packet* p) override;
+    int normalize_data(const uint8_t* ptr, const uint8_t* data_end, snort::Packet* p) override;
+#ifdef UNIT_TEST
+private:
+#endif  
     void decode_alert() override;
     void decompress_alert() override;
     void reset_state(snort::Flow* ssn) override;