From: Mike Stepanek (mstepane) Date: Mon, 21 Sep 2020 12:53:54 +0000 (+0000) Subject: Merge pull request #2480 in SNORT/snort3 from ~KATHARVE/snort3:h2i_bug to master X-Git-Tag: 3.0.3-1~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=500da46d48d56bcbe12f593265f3c5290be9c9c9;p=thirdparty%2Fsnort3.git Merge pull request #2480 in SNORT/snort3 from ~KATHARVE/snort3:h2i_bug to master Squashed commit of the following: commit 84f09f6257a9f9af151b8526c94166c713fbb134 Author: Katura Harvey Date: Tue Sep 8 12:03:09 2020 -0400 http2_inspect: fix how implement_reassemble uses frame_type --- diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc index 9b57ef382..df08d4c00 100644 --- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc +++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc @@ -288,8 +288,12 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio // We have the full frame header, compute some variables const uint32_t frame_length = get_frame_length(session_data-> scan_frame_header[source_id]); - const uint8_t type = session_data->frame_type[source_id] = get_frame_type( + const uint8_t type = get_frame_type( session_data->scan_frame_header[source_id]); + // Continuation frames are collapsed into the preceding Headers or Push Promise + // frame + if (type != FT_CONTINUATION) + session_data->frame_type[source_id] = type; const uint8_t frame_flags = get_frame_flags(session_data-> scan_frame_header[source_id]); const uint32_t old_stream = session_data->current_stream[source_id]; @@ -477,6 +481,7 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess // Copy headers if (session_data->use_leftover_hdr[source_id]) { + assert(session_data->frame_header_offset[source_id] == 0); memcpy(session_data->frame_header[source_id], session_data->leftover_hdr[source_id], FRAME_HEADER_LENGTH); session_data->frame_header_offset[source_id] += FRAME_HEADER_LENGTH; @@ -503,23 +508,24 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess session_data->remaining_frame_octets[source_id] = get_frame_length(session_data->frame_header[source_id] + session_data->frame_header_offset[source_id] - FRAME_HEADER_LENGTH); - uint8_t frame_flags = get_frame_flags(session_data->frame_header[source_id] + session_data->frame_header_offset[source_id] - FRAME_HEADER_LENGTH); - const uint8_t type = session_data->frame_type[source_id]; - if ((type == FT_DATA) || (type == FT_HEADERS)) + // Get the most recent frame header type + assert(session_data->frame_header_offset[source_id] >= FRAME_HEADER_LENGTH); + assert(session_data->frame_header_offset[source_id] % FRAME_HEADER_LENGTH == 0); + const uint8_t type = get_frame_type(session_data->frame_header[source_id] + + (session_data->frame_header_offset[source_id] - FRAME_HEADER_LENGTH)); + + // FIXIT-E Alert if padded flag is set but frame length is 0. Also alert if padded + // flag is set on an invalid frame type + if ((type == FT_HEADERS) and (frame_flags & PADDED) and + (get_frame_length(session_data->frame_header[source_id]) >= 1)) { - if ((frame_flags & PADDED) && - (get_frame_length(session_data->frame_header[source_id]) >= 1)) - { - session_data->get_padding_len[source_id] = true; - } + session_data->get_padding_len[source_id] = true; } } while (data_offset < len); - session_data->frame_type[source_id] = get_frame_type( - session_data->frame_header[source_id]); } if (flags & PKT_PDU_TAIL)