Merge pull request #2569 in SNORT/snort3 from ~NEHASH4/snort3:null_flow_crash to...

Squashed commit of the following:

commit fa300bfbf81b674b23c18de4ee80ffad10e9ec2d
Author: Neha Sharma <nehash4@cisco.com>
Date:   Fri Oct 23 03:01:20 2020 -0400

    dce_rpc: fixed incorrect accessing of FileFlows while pruning the flow

diff --git a/src/file_api/file_flows.cc b/src/file_api/file_flows.cc
index 03035494dda4384c215df95dfbd97e02a6029244..cc6c0090b29405a21ed44aedf2f6ea042dafd41c 100644 (file)
--- a/src/file_api/file_flows.cc
+++ b/src/file_api/file_flows.cc
@@ -83,12 +83,12 @@ void FileFlows::handle_retransmit(Packet* p)
     file->log_file_event(flow, file_policy);
 }
 
-FileFlows* FileFlows::get_file_flows(Flow* flow)
+FileFlows* FileFlows::get_file_flows(Flow* flow, bool to_create)
 {
 
     FileFlows* fd = (FileFlows*)flow->get_flow_data(FileFlows::file_flow_data_id);
 
-    if (fd)
+    if (!to_create or fd)
         return fd;
 
     FileInspect* fi = (FileInspect*)InspectorManager::get_inspector(FILE_ID_NAME, true);

diff --git a/src/file_api/file_flows.h b/src/file_api/file_flows.h
index ff17fcb113ce72269e87d57a22f5f1fcd9f1cfe5..f6321caa114bd827babd49b1e77beb3d28addaf2 100644 (file)
--- a/src/file_api/file_flows.h
+++ b/src/file_api/file_flows.h
@@ -63,7 +63,7 @@ public:
     void handle_retransmit(Packet*) override;
 
     // Factory method to get file flows
-    static FileFlows* get_file_flows(Flow*);
+    static FileFlows* get_file_flows(Flow*, bool to_create=true);
     static FilePolicyBase* get_file_policy(Flow*);
 
     FileContext* get_current_file_context();

diff --git a/src/service_inspectors/dce_rpc/dce_smb2.cc b/src/service_inspectors/dce_rpc/dce_smb2.cc
index 229a2f973b6eb09844300044607936cd2cd21e48..a53565f4de8e3008be7f8bd5da7a86c95273fe6c 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb2.cc
+++ b/src/service_inspectors/dce_rpc/dce_smb2.cc
@@ -103,7 +103,8 @@ DCE2_Smb2RequestTracker::~DCE2_Smb2RequestTracker()
 }
 
 DCE2_Smb2FileTracker::DCE2_Smb2FileTracker(uint64_t file_id_v, DCE2_Smb2TreeTracker* ttr_v,
-    DCE2_Smb2SessionTracker* str_v) : file_id(file_id_v), ttr(ttr_v), str(str_v)
+    DCE2_Smb2SessionTracker* str_v, Flow* flow_v) : file_id(file_id_v), ttr(ttr_v),
+    str(str_v), flow(flow_v)
 {
     debug_logf(dce_smb_trace, nullptr, "file tracker %" PRIu64 " created\n", file_id);
     memory::MemoryCap::update_allocations(sizeof(*this));
@@ -114,7 +115,8 @@ DCE2_Smb2FileTracker::~DCE2_Smb2FileTracker(void)
     debug_logf(dce_smb_trace, nullptr,
         "file tracker %" PRIu64 " file name hash %" PRIu64 " terminating\n",
          file_id, file_name_hash);
-    FileFlows* file_flows = FileFlows::get_file_flows(DetectionEngine::get_current_packet()->flow);
+
+    FileFlows* file_flows = FileFlows::get_file_flows(flow, false);
     if (file_flows)
     {
         file_flows->remove_processed_file_context(file_name_hash, file_id);

diff --git a/src/service_inspectors/dce_rpc/dce_smb2.h b/src/service_inspectors/dce_rpc/dce_smb2.h
index 8caa9f383f09e7783fd7a3afcb4c5d2d7bd41888..490d7f8acda879421a4872c39b53520bd1a41b1d 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb2.h
+++ b/src/service_inspectors/dce_rpc/dce_smb2.h
@@ -138,7 +138,7 @@ public:
     DCE2_Smb2FileTracker& operator=(const DCE2_Smb2FileTracker& arg) = delete;
 
     DCE2_Smb2FileTracker(uint64_t file_id_v, DCE2_Smb2TreeTracker* ttr_v,
-         DCE2_Smb2SessionTracker* str_v);
+         DCE2_Smb2SessionTracker* str_v, snort::Flow* flow_v);
     ~DCE2_Smb2FileTracker();
 
     bool ignore = false;
@@ -153,6 +153,7 @@ public:
     DCE2_SmbPduState smb2_pdu_state;
     DCE2_Smb2TreeTracker* ttr = nullptr;
     DCE2_Smb2SessionTracker* str = nullptr;
+    snort::Flow *flow = nullptr;
 };
 
 typedef DCE2_DbMap<uint64_t, DCE2_Smb2FileTracker*, std::hash<uint64_t> > DCE2_DbMapFtracker;

diff --git a/src/service_inspectors/dce_rpc/dce_smb2_commands.cc b/src/service_inspectors/dce_rpc/dce_smb2_commands.cc
index fb7d395f451a2bb4dfbd11acc3e3a588f6ae090b..a7a81672ff8d8eb330806f06c555f413bcba5be8 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb2_commands.cc
+++ b/src/service_inspectors/dce_rpc/dce_smb2_commands.cc
@@ -305,7 +305,7 @@ static void DCE2_Smb2CreateResponse(DCE2_Smb2SsnData*,
     DCE2_Smb2FileTracker* ftracker = ttr->findFtracker(fileId_persistent);
     if (!ftracker)
     {
-        ftracker = new DCE2_Smb2FileTracker(fileId_persistent, ttr, str);
+        ftracker = new DCE2_Smb2FileTracker(fileId_persistent, ttr, str, DetectionEngine::get_current_packet()->flow);
         ttr->insertFtracker(fileId_persistent, ftracker);
     }
     ftracker->file_name = rtracker->fname;
@@ -608,7 +608,7 @@ static void DCE2_Smb2ReadRequest(DCE2_Smb2SsnData* ssd,
     DCE2_Smb2FileTracker* ftracker =  ttr->findFtracker(fileId_persistent);
     if (!ftracker) // compounded create request + read request case
     {
-        ftracker = new DCE2_Smb2FileTracker(fileId_persistent, ttr, str);
+        ftracker = new DCE2_Smb2FileTracker(fileId_persistent, ttr, str, DetectionEngine::get_current_packet()->flow);
         ttr->insertFtracker(fileId_persistent, ftracker);
     }
 
@@ -761,7 +761,7 @@ static void DCE2_Smb2WriteRequest(DCE2_Smb2SsnData* ssd, const Smb2Hdr* smb_hdr,
     DCE2_Smb2FileTracker* ftracker = ttr->findFtracker(fileId_persistent);
     if (!ftracker) // compounded create request + write request case
     {
-        ftracker = new DCE2_Smb2FileTracker(fileId_persistent, ttr, str);
+        ftracker = new DCE2_Smb2FileTracker(fileId_persistent, ttr, str, DetectionEngine::get_current_packet()->flow);
         ttr->insertFtracker(fileId_persistent, ftracker);
     }
     if (!ftracker->ignore) // file tracker can not be nullptr here