From: Tom Peters Date: Wed, 8 Oct 2014 16:26:32 +0000 (-0400) Subject: reassembly buffer size always maximum X-Git-Tag: 3.0.0-233~1391 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3e70d972b2bfa98ef9971527bf6469f01b0ce93c;p=thirdparty%2Fsnort3.git reassembly buffer size always maximum --- diff --git a/src/service_inspectors/nhttp_inspect/nhttp_inspect.cc b/src/service_inspectors/nhttp_inspect/nhttp_inspect.cc index cf8db8137..9912a44e4 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_inspect.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_inspect.cc @@ -129,10 +129,11 @@ ProcessResult NHttpInspect::process(const uint8_t* data, const uint16_t dsize, F if (NHttpTestManager::use_test_output()) { msg_section->print_section(NHttpTestManager::get_output_file()); + fflush(NHttpTestManager::get_output_file()); if (NHttpTestManager::use_test_input()) { printf("Finished processing section from test %" PRIi64 "\n", NHttpTestManager::get_test_number()); + fflush(stdout); } - fflush(nullptr); } return return_value; diff --git a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc index 4d6aee891..d5dbbdb00 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc @@ -76,21 +76,6 @@ void NHttpStreamSplitter::prepare_flush(NHttpFlowData* session_data, uint32_t* f session_data->header_octets_visible[source_id] = 0; } -// Convenience function. Size buffer required to reassemble the current section plus possible aggregation. -uint32_t NHttpStreamSplitter::size_buffer_needed(unsigned total, NHttpEnums::SectionType type, - uint32_t possible_additional) { - switch (type) { - case SEC_CHUNK: - return 16384; - case SEC_REQUEST: - case SEC_STATUS: - case SEC_HEADER: - return total + possible_additional; - default: - return total; - } -} - StreamSplitter::Status NHttpStreamSplitter::scan (Flow* flow, const uint8_t* data, uint32_t length, uint32_t, uint32_t* flush_offset) { @@ -202,8 +187,10 @@ StreamSplitter::Status NHttpStreamSplitter::scan (Flow* flow, const uint8_t* dat } } -const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, unsigned offset, const uint8_t* data, - unsigned len, uint32_t flags, unsigned& copied) +// FIXIT-P total is not used because it is not reliably correct. Could be used to compute required buffer size +// instead of always allocating the maximum +const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned /* total */, unsigned offset, + const uint8_t* data, unsigned len, uint32_t flags, unsigned& copied) { static THREAD_LOCAL StreamBuffer nhttp_buf; @@ -224,12 +211,8 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, } data = test_buffer; offset = 0; - total = len; } - assert(total <= 63780); - assert(offset+len <= total); - // FIXIT-P stream should be enhanced to do discarding for us // For now flush-then-discard here is how scan() handles things we don't need to examine. if ((session_data->section_type[source_id] == SEC_DISCARD) || @@ -237,6 +220,7 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, if (NHttpTestManager::use_test_output()) { fprintf(NHttpTestManager::get_output_file(), "%s %u octets\n\n", (session_data->section_type[source_id] == SEC_DISCARD) ? "Discarded" : "Aborted", len); + fflush(NHttpTestManager::get_output_file()); } return nullptr; } @@ -249,13 +233,11 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, int32_t& buffer_length = !is_chunk ? session_data->section_buffer_length[source_id] : chunk_buffer_length; if (buffer == nullptr) { - buffer = new uint8_t[size_buffer_needed(total, session_data->section_type[source_id], - session_data->unused_octets_visible[source_id])]; + buffer = new uint8_t[63780]; } uint32_t num_excess = session_data->num_excess[source_id]; - unsigned num_to_copy = (len <= total - offset - num_excess) ? len : total - offset - num_excess; - memcpy(buffer + buffer_length + offset, data, num_to_copy); + memcpy(buffer + buffer_length + offset, data, len); if (flags & PKT_PDU_TAIL) { ProcessResult send_to_detection; if (!is_chunk) { @@ -292,6 +274,7 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, buffer_length = 0; if (NHttpTestManager::use_test_output()) { fprintf(NHttpTestManager::get_output_file(), "Sent to detection %u octets\n\n", nhttp_buf.length); + fflush(NHttpTestManager::get_output_file()); } return &nhttp_buf; case RES_IGNORE: diff --git a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.h b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.h index ecb978db4..3b2c35938 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.h +++ b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.h @@ -40,7 +40,7 @@ public: NHttpStreamSplitter(bool is_client_to_server, NHttpInspect* my_inspector_) : StreamSplitter(is_client_to_server), my_inspector(my_inspector_) { }; Status scan(Flow* flow, const uint8_t* data, uint32_t length, uint32_t not_used, uint32_t* flush_offset); - const StreamBuffer* reassemble(Flow* flow, unsigned total, unsigned offset, const uint8_t* data, unsigned len, + const StreamBuffer* reassemble(Flow* flow, unsigned /* total*/, unsigned offset, const uint8_t* data, unsigned len, uint32_t flags, unsigned& copied); bool is_paf() { return true; }; unsigned max() { return NHttpTestManager::use_test_input() ? 16384 : paf_max; }; @@ -49,7 +49,6 @@ private: NHttpEnums::SectionType section_type, bool tcp_close, uint64_t infractions, uint32_t num_octets, uint32_t length, uint32_t num_excess); void create_event(NHttpEnums::EventSid sid); - uint32_t size_buffer_needed(unsigned total, NHttpEnums::SectionType type, uint32_t possible_additional); NHttpInspect* const my_inspector; unsigned paf_max = 63780; }; diff --git a/src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt b/src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt index 37e6fcedb..dcc33dae5 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt +++ b/src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt @@ -38,7 +38,8 @@ # # This test tool does not implement the feature of being hardened against bad input. If you write a badly formatted or improper test case the # program may assert or crash. The responsibility is on the developer to get it right. Currently that is the best use of resources. - +# +# Test input is currently designed for single-threaded operation only. # *********************************************************************************************** # Valid response start lines @@ -760,6 +761,31 @@ fakeheader: 1234\r\n \r\n +@13005 +@break +@request + +GET / HTTP/1.0\r\n +\r\n + +@response + +HTTP/1.1 200 OK\r\n +Content-Type: text/plain; charset=iso-8859-1\r\n +Transfer-Encoding: chunked\r\n +\r\n + +0A\r\nABCDEFGHIJ\r\n + +0A\r\n1234567890\r\n + +0A\r\nabcdefghij\r\n + +0A\r\n9876543210\r\n + +0\r\n\r\n + + # *********************************************************************************************** # Alerts @14001