Merge pull request #1928 in SNORT/snort3 from ~DIPANDIT/snort3:port-CSCvg68807 to...

Squashed commit of the following:

commit 158c355b026dd0a57f139a129ac630e888b41a0c
Author: Dipto Pandit <dipandit@cisco.com>
Date:   Fri Jan 31 00:16:44 2020 -0500

    smb:Malware over size 131kb is not detected in SMBv2/SMBv3
    For SMB2/SMB3, the length field in NetBIOS Session Service Header should be considered 3 bytes.

diff --git a/src/service_inspectors/dce_rpc/dce_smb2.cc b/src/service_inspectors/dce_rpc/dce_smb2.cc
index 93604d1cca21820723d7761b41ce274007828af5..ec09665d51859cecfc091fcc9240af36ad541ba9 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb2.cc
+++ b/src/service_inspectors/dce_rpc/dce_smb2.cc
@@ -781,7 +781,7 @@ DCE2_SmbVersion DCE2_Smb2Version(const Packet* p)
 {
     /* Only check reassembled SMB2 packet*/
     if ( p->has_paf_payload() and
-        (p->dsize > sizeof(NbssHdr) + 4) ) // DCE2_SMB_ID is u32
+        (p->dsize > sizeof(NbssHdr) + DCE2_SMB_ID_SIZE) ) // DCE2_SMB_ID is u32
     {
         const Smb2Hdr* smb_hdr = (const Smb2Hdr*)(p->data + sizeof(NbssHdr));
         uint32_t smb_version_id = SmbId((const SmbNtHdr*)smb_hdr);

diff --git a/src/service_inspectors/dce_rpc/dce_smb_paf.cc b/src/service_inspectors/dce_rpc/dce_smb_paf.cc
index 945246b4589dc3a99eda9735c037cb335ca94442..84e09a4207580b0abbeea8198c86bcc8c6f2967b 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb_paf.cc
+++ b/src/service_inspectors/dce_rpc/dce_smb_paf.cc
@@ -45,10 +45,12 @@ using namespace snort;
  *          junk states, header type must be Session Message.
  *
  *********************************************************************/
-static inline bool DCE2_PafSmbIsValidNetbiosHdr(uint32_t nb_hdr, bool junk)
+static inline bool DCE2_PafSmbIsValidNetbiosHdr(uint32_t nb_hdr, bool junk, const SmbNtHdr *nt_hdr, uint32_t *nb_len)
 {
     uint8_t type = (uint8_t)(nb_hdr >> 24);
     uint8_t bit = (uint8_t)((nb_hdr & 0x00ff0000) >> 16);
+    bool is_smb1 = ( SmbId(nt_hdr) == DCE2_SMB2_ID ) ? false : true;
+    uint32_t nbs_hdr = 0;
 
     if (junk)
     {
@@ -70,9 +72,19 @@ static inline bool DCE2_PafSmbIsValidNetbiosHdr(uint32_t nb_hdr, bool junk)
             return false;
         }
     }
+    //The bit should be checked only for SMB1, because the length in NetBIOS header should not exceed 0x1FFFF.
+    //See [MS-SMB] 2.1 Transport. There is no such limit for SMB2 or SMB3
+    if(is_smb1)
+    {
+        if ((bit != 0x00) && (bit != 0x01))
+            return false;
+    }
+    nbs_hdr = htonl(nb_hdr);
 
-    if ((bit != 0x00) && (bit != 0x01))
-        return false;
+    if(is_smb1)
+        *nb_len = NbssLen((const NbssHdr *)&nbs_hdr);
+    else
+        *nb_len = NbssLen2((const NbssHdr *)&nbs_hdr);
 
     return true;
 }
@@ -97,8 +109,9 @@ static StreamSplitter::Status dce2_smb_paf(DCE2_PafSmbData* ss, Flow* flow, cons
 {
     uint32_t n = 0;
     StreamSplitter::Status ps = StreamSplitter::SEARCH;
-    uint32_t nb_hdr;
-    uint32_t nb_len;
+    const SmbNtHdr *nt_hdr = nullptr;
+    uint32_t nb_len = 0;
+
     DCE2_SmbSsnData* sd = get_dce2_smb_session_data(flow);
 
     if (dce2_paf_abort(flow, (DCE2_SsnData*)sd))
@@ -116,10 +129,11 @@ static StreamSplitter::Status dce2_smb_paf(DCE2_PafSmbData* ss, Flow* flow, cons
             break;
         case DCE2_PAF_SMB_STATES__3:
             DCE2_SMB_PAF_SHIFT(ss->nb_hdr, data[n]);
-            if (DCE2_PafSmbIsValidNetbiosHdr((uint32_t)ss->nb_hdr, false))
+            //(data + n + 1) points to the SMB header protocol identifier
+            //(0xFF,'SMB' or 0xFE,'SMB'), which follows the NetBIOS header
+            nt_hdr = (const SmbNtHdr *)(data + n + 1);
+            if (DCE2_PafSmbIsValidNetbiosHdr((uint32_t)ss->nb_hdr, false, nt_hdr, &nb_len))
             {
-                nb_hdr = htonl((uint32_t)ss->nb_hdr);
-                nb_len = NbssLen((const NbssHdr*)&nb_hdr);
                 *fp = (nb_len + sizeof(NbssHdr) + n) - ss->paf_state;
                 ss->paf_state = DCE2_PAF_SMB_STATES__0;
                 return StreamSplitter::FLUSH;
@@ -130,7 +144,12 @@ static StreamSplitter::Status dce2_smb_paf(DCE2_PafSmbData* ss, Flow* flow, cons
         case DCE2_PAF_SMB_STATES__7:
             DCE2_SMB_PAF_SHIFT(ss->nb_hdr, data[n]);
 
-            if (!DCE2_PafSmbIsValidNetbiosHdr((uint32_t)(ss->nb_hdr >> 32), true))
+            //(data + n - sizeof(DCE2_SMB_ID) + 1) points to the smb_idf field
+            //in SmbNtHdr (0xFF,'SMB' or 0xFE,'SMB'), which follows the NetBIOS header
+            nt_hdr = (const SmbNtHdr *)(data + n - DCE2_SMB_ID_SIZE + 1);
+            //ss->nb_hdr is the value to 4 bytes of NetBIOS header + 4 bytes of
+            //SMB header protocol identifier . Right shift by 32 bits to get the value of NetBIOS header
+            if (!DCE2_PafSmbIsValidNetbiosHdr((uint32_t)(ss->nb_hdr >> 32), true, nt_hdr, &nb_len))
             {
                 break;
             }
@@ -140,8 +159,6 @@ static StreamSplitter::Status dce2_smb_paf(DCE2_PafSmbData* ss, Flow* flow, cons
                 break;
             }
 
-            nb_hdr = htonl((uint32_t)(ss->nb_hdr >> 32));
-            nb_len = NbssLen((const NbssHdr*)&nb_hdr);
             *fp = (nb_len + sizeof(NbssHdr) + n) - ss->paf_state;
             ss->paf_state = DCE2_PAF_SMB_STATES__0;
 

diff --git a/src/service_inspectors/dce_rpc/smb_common.h b/src/service_inspectors/dce_rpc/smb_common.h
index 43ef674f6cdcc44717042e20685c6477f077e306..934ba9d79fadafced2e8f136ca8a0be935c3220b 100644 (file)
--- a/src/service_inspectors/dce_rpc/smb_common.h
+++ b/src/service_inspectors/dce_rpc/smb_common.h
@@ -72,6 +72,7 @@
 
 #define DCE2_SMB_ID   0xff534d42  /* \xffSMB */
 #define DCE2_SMB2_ID  0xfe534d42  /* \xfeSMB */
+#define DCE2_SMB_ID_SIZE 4
 
 // MS-FSCC Section 2.1.5 - Pathname
 #define DCE2_SMB_MAX_PATH_LEN  32760
@@ -374,13 +375,23 @@ struct SmbAndXCommon
     uint16_t smb_off2;     /* offset (from SMB hdr start) to next cmd (@smb_wct) */
 };
 
+//NbssLen should be used by SMB1
 inline uint32_t NbssLen(const NbssHdr* nb)
 {
-    /* Treat first bit of flags as the upper byte to length */
+    // Treat first bit of flags as the upper byte to length
+    //[MS-SMB] 2.1 Transport - Length can be maximum 0x1FFFF
     // The left operand of '&' is a garbage value
     return ((nb->flags & 0x01) << 16) | ntohs(nb->length);  // ... FIXIT-W
 }
 
+// NbssLen2 should be used by SMB2/SMB3
+inline uint32_t NbssLen2(const NbssHdr *nb)
+{
+    // The Length is 3 bytes. [MS-SMB2] 2.1 Transport
+    // The left operand of '<<' is a garbage value
+    return ((nb->flags << 16) | ntohs(nb->length));  // ... FIXIT-W
+}
+
 inline uint8_t NbssType(const NbssHdr* nb)
 {
     return nb->type;