From: George Koikara (gkoikara) Date: Tue, 25 Feb 2020 06:38:48 +0000 (+0000) Subject: Merge pull request #1928 in SNORT/snort3 from ~DIPANDIT/snort3:port-CSCvg68807 to... X-Git-Tag: 3.0.0-269~42 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f6b863d37ded4f361ae44a2e0cf840efbd2a8065;p=thirdparty%2Fsnort3.git Merge pull request #1928 in SNORT/snort3 from ~DIPANDIT/snort3:port-CSCvg68807 to master Squashed commit of the following: commit 158c355b026dd0a57f139a129ac630e888b41a0c Author: Dipto Pandit 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 93604d1cc..ec09665d5 100644 --- 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 945246b45..84e09a420 100644 --- 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 43ef674f6..934ba9d79 100644 --- 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;