From: Bhargava Jandhyala (bjandhya) Date: Sun, 11 Jul 2021 16:03:00 +0000 (+0000) Subject: Merge pull request #2972 in SNORT/snort3 from ~DIPANDIT/snort3:pinhole_fix to master X-Git-Tag: 3.1.8.0~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2307368f9b018b7d34647b144d4a8d893106afa0;p=thirdparty%2Fsnort3.git Merge pull request #2972 in SNORT/snort3 from ~DIPANDIT/snort3:pinhole_fix to master Squashed commit of the following: commit 846148ae043eb8d919ae152b08ab467f726d55f7 Author: Dipto Pandit (dipandit) Date: Fri Jul 9 12:09:48 2021 -0400 dce_rpc: fix crash when expected session comes after snort reload --- diff --git a/src/service_inspectors/dce_rpc/dce_smb2.cc b/src/service_inspectors/dce_rpc/dce_smb2.cc index 3b3619038..7ed8dfabc 100644 --- a/src/service_inspectors/dce_rpc/dce_smb2.cc +++ b/src/service_inspectors/dce_rpc/dce_smb2.cc @@ -250,8 +250,7 @@ void Dce2Smb2SessionData::process_command(const Smb2Hdr* smb_hdr, if (neg_resp_hdr->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL) { Packet* p = DetectionEngine::get_current_packet(); - Dce2SmbFlowData* fd = - create_expected_smb_flow_data(p, (dce2SmbProtoConf *)sd.config); + Dce2SmbFlowData* fd = create_expected_smb_flow_data(p); if (fd) { int result = Stream::set_snort_protocol_id_expected(p, PktType::TCP, @@ -462,18 +461,17 @@ void Dce2Smb2SessionData::process() const uint8_t* data_ptr = p->data; uint16_t data_len = p->dsize; - // Check header length - if (data_len < sizeof(NbssHdr) + SMB2_HEADER_LENGTH) - { - SMB_DEBUG(dce_smb_trace, DEFAULT_TRACE_OPTION_ID, TRACE_CRITICAL_LEVEL, - p, "Header error with data length %d\n",data_len); - dce2_smb_stats.v2_hdr_err++; - return; - } - // Process the header if (p->is_pdu_start()) { + // Check header length + if (data_len < sizeof(NbssHdr) + SMB2_HEADER_LENGTH) + { + SMB_DEBUG(dce_smb_trace, DEFAULT_TRACE_OPTION_ID, TRACE_CRITICAL_LEVEL, + p, "Header error with data length %d\n",data_len); + dce2_smb_stats.v2_hdr_err++; + return; + } const Smb2Hdr* smb_hdr = (const Smb2Hdr*)(data_ptr + sizeof(NbssHdr)); uint32_t next_command_offset; uint8_t compound_request_index = 0; diff --git a/src/service_inspectors/dce_rpc/dce_smb_common.cc b/src/service_inspectors/dce_rpc/dce_smb_common.cc index 026055772..83d2e22f3 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_common.cc +++ b/src/service_inspectors/dce_rpc/dce_smb_common.cc @@ -48,14 +48,6 @@ Dce2SmbFlowData::Dce2SmbFlowData(Dce2SmbSessionData* ssd_v) : FlowData(inspector ssd = ssd_v; } -void Dce2SmbFlowData::handle_expected(Packet* p) -{ - //we have a fd, but ssd not set, set it here in this flow - if (ssd) - delete ssd; - ssd = new Dce2Smb2SessionData(p, config); -} - Dce2SmbSessionData* Dce2SmbFlowData::upgrade(const Packet* p) { dce2SmbProtoConf* config = @@ -120,16 +112,27 @@ static inline DCE2_SmbVersion get_smb_version(const Packet* p) return DCE2_SMB_VERSION_NULL; } -Dce2SmbFlowData* create_expected_smb_flow_data(const Packet* p, dce2SmbProtoConf* config) +Dce2SmbFlowData* create_expected_smb_flow_data(const Packet* p) { DCE2_SmbVersion smb_version = get_smb_version(p); if (DCE2_SMB_VERSION_2 == smb_version) { - return new Dce2SmbFlowData(config); + return new Dce2SmbFlowData(); } return nullptr; } +Dce2SmbSessionData* create_smb_session_data(Dce2SmbFlowData* flow_data, const Packet* p, + dce2SmbProtoConf* config) +{ + DCE2_SmbVersion smb_version = get_smb_version(p); + if (DCE2_SMB_VERSION_2 != smb_version) + return nullptr; + Dce2SmbSessionData* ssd = (Dce2SmbSessionData*)new Dce2Smb2SessionData(p, config); + flow_data->update_smb_session_data(ssd); + return ssd; +} + Dce2SmbSessionData* create_new_smb_session(const Packet* p, dce2SmbProtoConf* config) { @@ -151,7 +154,8 @@ Dce2SmbSessionData* create_new_smb_session(const Packet* p, DCE2_SsnData* get_dce2_session_data(snort::Flow* flow) { Dce2SmbFlowData* fd = (Dce2SmbFlowData*)flow->get_flow_data(Dce2SmbFlowData::inspector_id); - return fd ? fd->get_smb_session_data()->get_dce2_session_data() : nullptr; + return fd ? fd->get_smb_session_data() ? fd->get_smb_session_data()->get_dce2_session_data() : + nullptr : nullptr; } inline FileContext* get_smb_file_context(const Packet* p) diff --git a/src/service_inspectors/dce_rpc/dce_smb_common.h b/src/service_inspectors/dce_rpc/dce_smb_common.h index 5ef7ce18c..f0a8bdb32 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_common.h +++ b/src/service_inspectors/dce_rpc/dce_smb_common.h @@ -270,12 +270,11 @@ class Dce2SmbFlowData : public snort::FlowData { public: Dce2SmbFlowData(Dce2SmbSessionData*); - Dce2SmbFlowData(dce2SmbProtoConf* cfg) : snort::FlowData(inspector_id) + Dce2SmbFlowData() : snort::FlowData(inspector_id) { dce2_smb_stats.concurrent_sessions++; if (dce2_smb_stats.max_concurrent_sessions < dce2_smb_stats.concurrent_sessions) dce2_smb_stats.max_concurrent_sessions = dce2_smb_stats.concurrent_sessions; - config = cfg; ssd = nullptr; } @@ -291,19 +290,24 @@ public: { return ssd; } Dce2SmbSessionData* upgrade(const snort::Packet*); + void update_smb_session_data(Dce2SmbSessionData* ssd_v) + { + if (ssd) delete ssd; + ssd = ssd_v; + } void handle_retransmit(snort::Packet*) override; - void handle_expected(snort::Packet*) override; public: static unsigned inspector_id; private: Dce2SmbSessionData* ssd; - dce2SmbProtoConf* config; }; -Dce2SmbFlowData* create_expected_smb_flow_data(const snort::Packet*, dce2SmbProtoConf*); +Dce2SmbFlowData* create_expected_smb_flow_data(const snort::Packet*); Dce2SmbSessionData* create_new_smb_session(const snort::Packet*, dce2SmbProtoConf*); +Dce2SmbSessionData* create_smb_session_data(Dce2SmbFlowData*, const snort::Packet*, + dce2SmbProtoConf*); DCE2_SsnData* get_dce2_session_data(snort::Flow*); snort::FileContext* get_smb_file_context(const snort::Packet*); snort::FileContext* get_smb_file_context(snort::Flow*, uint64_t, uint64_t, bool); diff --git a/src/service_inspectors/dce_rpc/dce_smb_inspector.cc b/src/service_inspectors/dce_rpc/dce_smb_inspector.cc index 1ce2430b2..9f0bd1e4b 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_inspector.cc +++ b/src/service_inspectors/dce_rpc/dce_smb_inspector.cc @@ -69,7 +69,14 @@ void Dce2Smb::eval(Packet* p) Dce2SmbSessionData* smb_session_data; if (smb_flowdata) + { smb_session_data = smb_flowdata->get_smb_session_data(); + // if flow data present but session data is not, it is an expected session + // try to update the session data, will only do for SMBv2. Ideally it should + // be done in handle_expected, but we dont have access to the config there. + if (!smb_session_data) + smb_session_data = create_smb_session_data(smb_flowdata, p, &config); + } else smb_session_data = create_new_smb_session(p, &config);