Pull request #3403: smb: handling file context cleanup

Merge in SNORT/snort3 from ~UMUNNIKR/snort3:smb_mem_corrupt_fix to master

Squashed commit of the following:

commit c7ce156cfa51ff1fda4d65b3b8f90c3783c77652
Author: Unnikrishnan M <umunnikr@cisco.com>
Date:   Mon May 2 10:19:50 2022 +0530

    smb: handling file context cleanup

diff --git a/src/service_inspectors/dce_rpc/dce_co.cc b/src/service_inspectors/dce_rpc/dce_co.cc
index 1f248bd3a3fd39bbb5a6555efcb3f097e888451f..69b65225ed3ed31177f6af7ded6cb9c957d83b46 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_co.cc
+++ b/src/service_inspectors/dce_rpc/dce_co.cc
@@ -334,7 +334,11 @@ static inline void DCE2_CoSetRdata(DCE2_CoTracker* cot, uint8_t* co_ptr, uint16_
         (cot->frag_tracker.opnum != DCE2_SENTINEL) ?
         (uint16_t)cot->frag_tracker.opnum : (uint16_t)cot->opnum;
 
-    if ( DetectionEngine::get_current_packet()->is_from_client() )
+    Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return;
+
+    if ( p->is_from_client() )
     {
         DceRpcCoRequest* co_req = (DceRpcCoRequest*)((uint8_t*)co_hdr + sizeof(DceRpcCoHdr));
         /* Doesn't really matter if this wraps ... it is basically just for presentation */
@@ -380,7 +384,10 @@ void DCE2_CoInitRdata(uint8_t* co_ptr, int dir)
 
 static inline DCE2_CoSeg* DCE2_CoGetSegPtr(DCE2_CoTracker* cot)
 {
-    if ( DetectionEngine::get_current_packet()->is_from_server() )
+    Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return nullptr;
+    if ( p->is_from_server() )
         return &cot->srv_seg;
 
     return &cot->cli_seg;
@@ -556,7 +563,11 @@ static DCE2_Ret DCE2_CoHdrChecks(DCE2_SsnData* sd, DCE2_CoTracker* cot, const Dc
         return DCE2_RET__ERROR;
     }
 
-    if (DetectionEngine::get_current_packet()->is_from_client() && (cot->max_xmit_frag != DCE2_SENTINEL))
+    Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return DCE2_RET__ERROR;
+
+    if (p->is_from_client() && (cot->max_xmit_frag != DCE2_SENTINEL))
     {
         if (frag_len > cot->max_xmit_frag)
         {
@@ -1167,7 +1178,11 @@ static int DCE2_CoGetAuthLen(DCE2_SsnData* sd, const DceRpcCoHdr* co_hdr,
 
 static DCE2_Buffer* DCE2_CoGetFragBuf(DCE2_CoFragTracker* ft)
 {
-    if ( DetectionEngine::get_current_packet()->is_from_server() )
+    Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return nullptr;
+
+    if ( p->is_from_server() )
         return ft->srv_stub_buf;
 
     return ft->cli_stub_buf;
@@ -1245,6 +1260,9 @@ static Packet* DCE2_CoGetRpkt(DCE2_SsnData* sd, DCE2_CoTracker* cot,
     uint32_t frag_len = 0, seg_len = 0;
     Packet* rpkt = nullptr;
 
+    if (seg_buf == nullptr)
+        return nullptr;
+
     *rtype = DCE2_RPKT_TYPE__NULL;
 
     switch (co_rtype)
@@ -1342,7 +1360,10 @@ static Packet* dce_co_reassemble(DCE2_SsnData* sd, DCE2_CoTracker* cot,
     DCE2_CoRpktType co_rtype, const DceRpcCoHdr** co_hdr)
 {
     dce2CommonStats* dce_common_stats = dce_get_proto_stats_ptr(sd);
-    bool from_client = DetectionEngine::get_current_packet()->is_from_client();
+    Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return nullptr;
+    bool from_client = p->is_from_client();
 
     int co_hdr_len = from_client ? DCE2_MOCK_HDR_LEN__CO_CLI : DCE2_MOCK_HDR_LEN__CO_SRV;
     int smb_hdr_len = from_client ? DCE2_MOCK_HDR_LEN__SMB_CLI : DCE2_MOCK_HDR_LEN__SMB_SRV;
@@ -1450,6 +1471,8 @@ static DCE2_Ret dce_co_handle_frag(DCE2_SsnData* sd, DCE2_CoTracker* cot,
     DCE2_Ret status;
     dce2CommonStats* dce_common_stats = dce_get_proto_stats_ptr(sd);
     Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return DCE2_RET__ERROR;
 
     if ( p->is_from_client() )
     {
@@ -1852,6 +1875,8 @@ static void DCE2_CoResponse(DCE2_SsnData* sd, DCE2_CoTracker* cot,
     cot->call_id = DceRpcCoCallId(co_hdr);
 
     Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return;
 
     if (DceRpcCoFirstFrag(co_hdr) && DceRpcCoLastFrag(co_hdr))
     {
@@ -1928,13 +1953,16 @@ static void DCE2_CoDecode(DCE2_SsnData* sd, DCE2_CoTracker* cot,
     const DceRpcCoHdr* co_hdr = (const DceRpcCoHdr*)frag_ptr;
     int pdu_type = DceRpcCoPduType(co_hdr);
     dce2CommonStats* dce_common_stats = dce_get_proto_stats_ptr(sd);
+    Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return;
 
     /* We've got the main header.  Move past it to the
      * start of the pdu */
     dce2_move(frag_ptr, frag_len, sizeof(DceRpcCoHdr));
 
     /* Client specific pdu types - some overlap with server */
-    if ( DetectionEngine::get_current_packet()->is_from_client() )
+    if ( p->is_from_client() )
     {
         switch (pdu_type)
         {
@@ -2137,8 +2165,9 @@ static DCE2_Ret DCE2_CoSegEarlyRequest(DCE2_CoTracker* cot,
 static void DCE2_CoEarlyReassemble(DCE2_SsnData* sd, DCE2_CoTracker* cot)
 {
     DCE2_Buffer* frag_buf = DCE2_CoGetFragBuf(&cot->frag_tracker);
+    Packet* p = DetectionEngine::get_current_packet();
 
-    if ( DetectionEngine::get_current_packet()->is_from_server() )
+    if ( (p == nullptr) || p->is_from_server() )
         return;
 
     if (!DCE2_BufferIsEmpty(frag_buf))
@@ -2206,8 +2235,11 @@ static Packet* DCE2_CoGetSegRpkt(DCE2_SsnData* sd,
     const uint8_t* data_ptr, uint32_t data_len)
 {
     Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return nullptr;
     Packet* rpkt = nullptr;
-    int smb_hdr_len = p->is_from_client() ? DCE2_MOCK_HDR_LEN__SMB_CLI : DCE2_MOCK_HDR_LEN__SMB_SRV;
+    int smb_hdr_len = p->is_from_client() ? DCE2_MOCK_HDR_LEN__SMB_CLI :
+        DCE2_MOCK_HDR_LEN__SMB_SRV;
 
     switch (sd->trans)
     {
@@ -2249,7 +2281,11 @@ static void DCE2_CoSegDecode(DCE2_SsnData* sd, DCE2_CoTracker* cot, DCE2_CoSeg*
     dce2CommonStats* dce_common_stats = dce_get_proto_stats_ptr(sd);
     int smb_hdr_len;
 
-    if ( DetectionEngine::get_current_packet()->is_from_client() )
+    Packet* p = DetectionEngine::get_current_packet();
+    if (p == nullptr)
+        return;
+
+    if ( p->is_from_client() )
     {
         smb_hdr_len = DCE2_MOCK_HDR_LEN__SMB_CLI;
         dce_common_stats->co_cli_seg_reassembled++;
@@ -2394,6 +2430,9 @@ void DCE2_CoProcess(DCE2_SsnData* sd, DCE2_CoTracker* cot,
     dce2CommonStats* dce_common_stats = dce_get_proto_stats_ptr(sd);
     uint32_t num_frags = 0;
 
+    if (seg == nullptr)
+        return;
+
     dce_common_stats->co_pdus++;
     co_reassembled = 0;
 

diff --git a/src/service_inspectors/dce_rpc/dce_smb2.cc b/src/service_inspectors/dce_rpc/dce_smb2.cc
index 41ef5129e4babc30f97cda82046a42279650e0bb..cabb7a531978552c93942a777e1e0c5dd631750a 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb2.cc
+++ b/src/service_inspectors/dce_rpc/dce_smb2.cc
@@ -115,6 +115,7 @@ Dce2Smb2SessionData::~Dce2Smb2SessionData(void)
     for (auto it_session : connected_sessions)
     {
         it_session.second->detach_flow(flow_key);
+        it_session.second->set_file_context_cleaned(true);
     }
 
     if (get_tcp_file_tracker() && !(get_tcp_file_tracker()->get_flow_state_map().empty()))

diff --git a/src/service_inspectors/dce_rpc/dce_smb2_session.cc b/src/service_inspectors/dce_rpc/dce_smb2_session.cc
index a2665b267275959392b6e1c6b02eb0b891fcbb93..55b25f0a37338cb413ed68f7eaeb1503065eac70 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb2_session.cc
+++ b/src/service_inspectors/dce_rpc/dce_smb2_session.cc
@@ -159,6 +159,12 @@ void Dce2Smb2SessionTracker::clean_file_context_from_flow(uint64_t file_id, uint
     attached_flows_mutex.lock();
     for (auto it_flow : attached_flows)
     {
+        if (get_file_context_cleaned())
+        {
+            attached_flows_mutex.unlock();
+            set_do_not_delete(false);
+            return;
+        }
         snort::FileFlows* file_flows = snort::FileFlows::get_file_flows(
             it_flow.second->get_tcp_flow(), false);
         if (file_flows)

diff --git a/src/service_inspectors/dce_rpc/dce_smb2_session.h b/src/service_inspectors/dce_rpc/dce_smb2_session.h
index 382347e5eb771092ec7d1df14296d475c674101e..8a813e1059183bbde0ab1f437879ad27c4099164 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb2_session.h
+++ b/src/service_inspectors/dce_rpc/dce_smb2_session.h
@@ -37,6 +37,7 @@ public:
         session_key = key;
         reload_prune = false;
         do_not_delete = false;
+        file_context_cleaned = false;
         command_prev = SMB2_COM_MAX;
         encryption_flag = false;
         SMB_DEBUG(dce_smb_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, GET_CURRENT_PACKET, 
@@ -77,6 +78,8 @@ public:
     uint64_t get_session_id() { return session_id; }
     void set_do_not_delete(bool flag) { do_not_delete = flag; }
     bool get_do_not_delete() { return do_not_delete; }
+    void set_file_context_cleaned(bool flag) { file_context_cleaned = flag; }
+    bool get_file_context_cleaned() { return file_context_cleaned; }
     void set_prev_comand(uint16_t cmd) { command_prev = cmd; }
     uint16_t get_prev_command() { return command_prev; }
     std::mutex co_tracker_mutex;
@@ -91,6 +94,7 @@ private:
     // do_not_delete is to make sure when we are in processing we should not delete the context
     // which is being processed
     bool do_not_delete;
+    bool file_context_cleaned;
     Dce2Smb2TreeTracker* find_tree_for_message(const uint64_t, const uint32_t);
     uint64_t session_id;
     //to keep the tab of previous command

diff --git a/src/service_inspectors/dce_rpc/dce_smb_utils.cc b/src/service_inspectors/dce_rpc/dce_smb_utils.cc
index 1052f5e3d29a9abff77544929c57fbf8186baf3b..521f2d5db74c60542591b8a7be692622d0ac50f1 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb_utils.cc
+++ b/src/service_inspectors/dce_rpc/dce_smb_utils.cc
@@ -1722,6 +1722,7 @@ void DCE2_SmbProcessFileData(DCE2_SmbSsnData* ssd,
     {
         set_file_data(data_ptr, (data_len > UINT16_MAX) ? UINT16_MAX : (uint16_t)data_len);
         DCE2_FileDetect();
+        set_file_data(nullptr, 0);
     }
 
     if (ftracker == ssd->fapi_ftracker)

diff --git a/src/service_inspectors/dce_rpc/smb_message.cc b/src/service_inspectors/dce_rpc/smb_message.cc
index d1de8a410f8da22db439704e5f17df327192d7f0..b7082d25c5e7aacd06808f21cc1624eeaca0b0fb 100644 (file)
--- a/src/service_inspectors/dce_rpc/smb_message.cc
+++ b/src/service_inspectors/dce_rpc/smb_message.cc
@@ -1725,6 +1725,7 @@ void DCE2_Smb1Process(DCE2_SmbSsnData* ssd)
 
             if (!DCE2_BufferIsEmpty(*seg_buf))
             {
+                set_file_data(nullptr, 0);
                 DCE2_BufferDestroy(*seg_buf);
                 *seg_buf = nullptr;
             }