From: Mike Stepanek (mstepane) Date: Mon, 24 Feb 2020 14:07:49 +0000 (+0000) Subject: Merge pull request #2030 in SNORT/snort3 from ~THOPETER/snort3:nhttp135 to master X-Git-Tag: 3.0.0-269~44 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=584660beb49de8010714a8c23c674899f2991af7;p=thirdparty%2Fsnort3.git Merge pull request #2030 in SNORT/snort3 from ~THOPETER/snort3:nhttp135 to master Squashed commit of the following: commit d7b1e4a922555e1d5b046eaacb8f36849e56e1ac Author: Tom Peters Date: Fri Feb 21 11:26:22 2020 -0500 http_inspect: improve precautions for stream interactions --- diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h index e4fe48159..13eb602c0 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.h +++ b/src/service_inspectors/http_inspect/http_flow_data.h @@ -103,6 +103,7 @@ private: // *** StreamSplitter => Inspector (facts about the most recent message section) HttpEnums::SectionType section_type[2] = { HttpEnums::SEC__NOT_COMPUTE, HttpEnums::SEC__NOT_COMPUTE }; + int32_t octets_reassembled[2] = { HttpCommon::STAT_NOT_PRESENT, HttpCommon::STAT_NOT_PRESENT }; int32_t num_head_lines[2] = { HttpCommon::STAT_NOT_PRESENT, HttpCommon::STAT_NOT_PRESENT }; bool tcp_close[2] = { false, false }; bool partial_flush[2] = { false, false }; diff --git a/src/service_inspectors/http_inspect/http_inspect.cc b/src/service_inspectors/http_inspect/http_inspect.cc index f6f6f2cfa..e2f43fd85 100644 --- a/src/service_inspectors/http_inspect/http_inspect.cc +++ b/src/service_inspectors/http_inspect/http_inspect.cc @@ -359,9 +359,15 @@ void HttpInspect::eval(Packet* p) HttpFlowData* session_data = http_get_flow_data(p->flow); - // FIXIT-H Workaround for unexpected eval() calls - if (session_data->section_type[source_id] == SEC__NOT_COMPUTE) + // FIXIT-H Workaround for unexpected eval() calls. Convert to asserts when possible. + if ((session_data->section_type[source_id] == SEC__NOT_COMPUTE) || + (session_data->type_expected[source_id] == SEC_ABORT) || + (session_data->octets_reassembled[source_id] != p->dsize)) + { + session_data->type_expected[source_id] = SEC_ABORT; return; + } + session_data->octets_reassembled[source_id] = STAT_NOT_PRESENT; // Don't make pkt_data for headers available to detection if ((session_data->section_type[source_id] == SEC_HEADER) || @@ -415,7 +421,6 @@ bool HttpInspect::process(const uint8_t* data, const uint16_t dsize, Flow* const { HttpMsgSection* current_section; HttpFlowData* session_data = http_get_flow_data(flow); - assert(session_data != nullptr); if (!session_data->partial_flush[source_id]) HttpModule::increment_peg_counts(PEG_INSPECT); diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc index c930070d9..6f4bc4aac 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc @@ -168,21 +168,13 @@ bool HttpStreamSplitter::init_partial_flush(Flow* flow) { Profile profile(HttpModule::get_profile_stats()); - if (source_id != SRC_SERVER) - { - assert(false); - return false; - } + assert(source_id == SRC_SERVER); HttpFlowData* session_data = HttpInspect::http_get_flow_data(flow); assert(session_data != nullptr); - if ((session_data->type_expected[source_id] != SEC_BODY_CL) && - (session_data->type_expected[source_id] != SEC_BODY_OLD) && - (session_data->type_expected[source_id] != SEC_BODY_CHUNK)) - { - assert(false); - return false; - } + assert((session_data->type_expected[source_id] == SEC_BODY_CL) || + (session_data->type_expected[source_id] == SEC_BODY_OLD) || + (session_data->type_expected[source_id] == SEC_BODY_CHUNK)); #ifdef REG_TEST if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP) && diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc index fb979d741..2ddc5927c 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc @@ -276,6 +276,12 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total, } #endif + assert(session_data->type_expected[source_id] != SEC_ABORT); + if (session_data->section_type[source_id] == SEC__NOT_COMPUTE) + { + return { nullptr, 0 }; + } + // Sometimes it is necessary to reassemble zero bytes when a connection is closing to trigger // proper clean up. But even a zero-length buffer cannot be processed with a nullptr lest we // get in trouble with memcpy() (undefined behavior) or some library. @@ -283,13 +289,6 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total, if (data == nullptr) data = (const uint8_t*)""; - // FIXIT-H Workaround for TP Bug 149662 - if (session_data->section_type[source_id] == SEC__NOT_COMPUTE) - { - return { nullptr, 0 }; - } - - assert(session_data->section_type[source_id] != SEC__NOT_COMPUTE); uint8_t*& partial_buffer = session_data->partial_buffer[source_id]; uint32_t& partial_buffer_length = session_data->partial_buffer_length[source_id]; uint32_t& partial_raw_bytes = session_data->partial_raw_bytes[source_id]; @@ -424,6 +423,7 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total, http_buf.data = buffer; http_buf.length = buf_size; + session_data->octets_reassembled[source_id] = buf_size; buffer = nullptr; session_data->section_offset[source_id] = 0;