From: Mike Stepanek (mstepane) Date: Mon, 5 Oct 2020 20:30:20 +0000 (+0000) Subject: Merge pull request #2514 in SNORT/snort3 from ~KATHARVE/snort3:fix_padding to master X-Git-Tag: 3.0.3-2~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4f87adb305d3944b019a2537ffe391d1b1368a64;p=thirdparty%2Fsnort3.git Merge pull request #2514 in SNORT/snort3 from ~KATHARVE/snort3:fix_padding to master Squashed commit of the following: commit e6e7fc65e4a104851bf523a427a3186b71d26197 Author: Katura Harvey Date: Sun Sep 27 15:36:22 2020 -0400 http2_inspect: fix frame padding handling --- diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.cc b/src/service_inspectors/http2_inspect/http2_data_cutter.cc index b6e6877c3..61b4a1940 100644 --- a/src/service_inspectors/http2_inspect/http2_data_cutter.cc +++ b/src/service_inspectors/http2_inspect/http2_data_cutter.cc @@ -39,8 +39,8 @@ Http2DataCutter::Http2DataCutter(Http2FlowData* _session_data, HttpCommon::Sourc // Scan data frame, extract information needed for http scan. // http scan will need the data only, stripped of padding and header. -bool Http2DataCutter::http2_scan(const uint8_t* data, uint32_t length, - uint32_t* flush_offset, uint32_t frame_len, uint8_t flags, uint32_t& data_offset) +bool Http2DataCutter::http2_scan(uint32_t length, uint32_t* flush_offset, uint32_t frame_len, + uint8_t flags, uint32_t& data_offset) { cur_data_offset = data_offset; cur_data = cur_padding = 0; @@ -50,12 +50,20 @@ bool Http2DataCutter::http2_scan(const uint8_t* data, uint32_t length, padding_len = data_bytes_read = padding_read = 0; frame_flags = flags; *flush_offset = frame_bytes_seen = FRAME_HEADER_LENGTH; - if (frame_length == 0) - data_state = FULL_FRAME; - else if ((frame_flags & PADDED) !=0) - data_state = PADDING_LENGTH; - else + + if (frame_flags & PADDED) + { + padding_len = session_data->padding_length[source_id]; + data_len -= (padding_len + 1); + frame_bytes_seen += 1; + } + + if (data_len) data_state = DATA; + else if (padding_len) + data_state = PADDING; + else + data_state = FULL_FRAME; } uint32_t cur_pos = data_offset + leftover_bytes + leftover_padding; @@ -63,26 +71,6 @@ bool Http2DataCutter::http2_scan(const uint8_t* data, uint32_t length, { switch (data_state) { - case PADDING_LENGTH: - padding_len = *(data + cur_data_offset); - - if (data_len <= padding_len) - { - *session_data->infractions[source_id] += INF_PADDING_LEN; - session_data->events[source_id]->create_event(EVENT_PADDING_LEN); - return false; - } - data_len -= (padding_len + 1); - - if (data_len) - data_state = DATA; - else if (padding_len) - data_state = PADDING; - else - data_state = FULL_FRAME; - cur_pos++; - cur_data_offset++; - break; case DATA: { const uint32_t missing = data_len - data_bytes_read; @@ -113,6 +101,7 @@ bool Http2DataCutter::http2_scan(const uint8_t* data, uint32_t length, if (data_state == FULL_FRAME) session_data->reading_frame[source_id] = false; + //FIXIT-E shouldn't need both scan_remaining_frame_octets and frame_bytes_seen frame_bytes_seen += (cur_pos - leftover_bytes - data_offset - leftover_padding); *flush_offset = data_offset = cur_pos; session_data->scan_remaining_frame_octets[source_id] = frame_length - frame_bytes_seen; @@ -143,8 +132,6 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t* else leftover_padding = 0; *flush_offset -= leftover_bytes + leftover_padding; - if (leftover_bytes || data_state != FULL_FRAME) - session_data->mid_data_frame[source_id] = true; } else if (scan_result == StreamSplitter::SEARCH) { @@ -163,9 +150,9 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t* if (leftover_bytes == 0) { // Done with this frame, cleanup - session_data->mid_data_frame[source_id] = false; session_data->scan_octets_seen[source_id] = 0; session_data->scan_remaining_frame_octets[source_id] = 0; + session_data->scan_state[source_id] = SCAN_HEADER; frame_bytes_seen = 0; if (frame_flags & END_STREAM) @@ -195,7 +182,7 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t* StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t length, uint32_t* flush_offset, uint32_t& data_offset, uint32_t frame_len, uint8_t frame_flags) { - if (!http2_scan(data, length, flush_offset, frame_len, frame_flags, data_offset)) + if (!http2_scan(length, flush_offset, frame_len, frame_flags, data_offset)) return StreamSplitter::ABORT; session_data->stream_in_hi = session_data->current_stream[source_id]; diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.h b/src/service_inspectors/http2_inspect/http2_data_cutter.h index 80e0146b3..1451e7c23 100644 --- a/src/service_inspectors/http2_inspect/http2_data_cutter.h +++ b/src/service_inspectors/http2_inspect/http2_data_cutter.h @@ -45,7 +45,7 @@ private: // total per frame - scan uint32_t frame_length; uint32_t data_len; - uint32_t padding_len; + uint8_t padding_len; uint8_t frame_flags; // accumulating - scan uint32_t frame_bytes_seen = 0; @@ -74,15 +74,15 @@ private: // // data scan - enum DataState { PADDING_LENGTH, DATA, PADDING, FULL_FRAME }; + enum DataState { DATA, PADDING, FULL_FRAME }; enum DataState data_state; // reassemble enum ReassembleState { GET_FRAME_HDR, GET_PADDING_LEN, SEND_DATA, SKIP_PADDING, CLEANUP }; enum ReassembleState reassemble_state = GET_FRAME_HDR; - bool http2_scan(const uint8_t* data, uint32_t length, uint32_t* flush_offset, - uint32_t frame_len, uint8_t frame_flags, uint32_t& data_offset); + bool http2_scan(uint32_t length, uint32_t* flush_offset, uint32_t frame_len, + uint8_t frame_flags, uint32_t& data_offset); snort::StreamSplitter::Status http_scan(const uint8_t* data, uint32_t* flush_offset); }; diff --git a/src/service_inspectors/http2_inspect/http2_enum.h b/src/service_inspectors/http2_inspect/http2_enum.h index 515bc2b31..d30ab3771 100644 --- a/src/service_inspectors/http2_inspect/http2_enum.h +++ b/src/service_inspectors/http2_inspect/http2_enum.h @@ -73,6 +73,8 @@ enum EventSid EVENT_PSEUDO_HEADER_IN_TRAILERS = 18, EVENT_INVALID_PSEUDO_HEADER = 19, EVENT_TRAILERS_NOT_END = 20, + EVENT_PADDING_ON_INVALID_FRAME = 21, + EVENT_PADDING_ON_EMPTY_FRAME = 22, EVENT__MAX_VALUE }; @@ -111,6 +113,8 @@ enum Infraction INF_UNUSED_2 = 28, INF_PSEUDO_HEADER_IN_TRAILERS = 29, INF_TRAILERS_NOT_END = 30, + INF_PADDING_ON_INVALID_FRAME = 31, + INF_PADDING_ON_EMPTY_FRAME = 32, INF__MAX_VALUE }; @@ -133,6 +137,7 @@ enum SettingsFrameIds MAX_HEADER_LIST_SIZE, }; +enum ScanState { SCAN_HEADER, SCAN_PADDING_LENGTH, SCAN_DATA, SCAN_EMPTY_DATA }; } // end namespace Http2Enums #endif diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h index 98fc44835..f3e5f4972 100644 --- a/src/service_inspectors/http2_inspect/http2_flow_data.h +++ b/src/service_inspectors/http2_inspect/http2_flow_data.h @@ -145,8 +145,9 @@ protected: uint8_t scan_frame_header[2][Http2Enums::FRAME_HEADER_LENGTH]; uint32_t scan_remaining_frame_octets[2] = { 0, 0 }; uint32_t scan_octets_seen[2] = { 0, 0 }; - bool mid_data_frame[2] = { false, false }; //set for data frame with multiple flushes bool data_processing[2] = { false, false }; + uint8_t padding_length[2] = { 0, 0 }; + Http2Enums::ScanState scan_state[2] = { Http2Enums::SCAN_HEADER, Http2Enums::SCAN_HEADER }; // Scan signals to reassemble() bool payload_discard[2] = { false, false }; @@ -166,8 +167,8 @@ protected: uint32_t frame_header_offset[2] = { 0, 0 }; uint32_t frame_data_offset[2] = { 0, 0 }; uint32_t remaining_frame_octets[2] = { 0, 0 }; - uint8_t padding_octets_in_frame[2] = { 0, 0 }; - bool get_padding_len[2] = { false, false }; + uint8_t remaining_padding_octets_in_frame[2] = { 0, 0 }; + bool read_padding_len[2] = { false, false }; // used to signal frame wasn't fully read yet, // currently used by payload injector diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter.h b/src/service_inspectors/http2_inspect/http2_stream_splitter.h index 3e7384460..96c071550 100644 --- a/src/service_inspectors/http2_inspect/http2_stream_splitter.h +++ b/src/service_inspectors/http2_inspect/http2_stream_splitter.h @@ -48,15 +48,18 @@ public: private: const HttpCommon::SourceId source_id; - - static StreamSplitter::Status data_scan(Http2FlowData* session_data, const uint8_t* data, - uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id, - uint32_t frame_length, uint8_t frame_flags, uint32_t& data_offset); - static void partial_flush_data(Http2FlowData* session_data, HttpCommon::SourceId source_id, - uint32_t* flush_offset, uint32_t data_offset, Http2Stream* const stream); + static StreamSplitter::Status data_frame_header_checks(Http2FlowData* session_data, + uint32_t* flush_offset, HttpCommon::SourceId source_id, uint32_t frame_length, + uint32_t& data_offset); + static StreamSplitter::Status non_data_frame_header_checks( + Http2FlowData* session_data, HttpCommon::SourceId source_id, uint32_t frame_length, + uint8_t type); static StreamSplitter::Status non_data_scan(Http2FlowData* session_data, - uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id, - uint32_t frame_length, uint8_t type, uint8_t frame_flags, uint32_t& data_offset); + uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id, uint8_t type, + uint8_t frame_flags, uint32_t& data_offset); + static void partial_flush_data(Http2FlowData* session_data, + HttpCommon::SourceId source_id, uint32_t* flush_offset, uint32_t data_offset, + Http2Stream* const stream); static snort::StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t* data, uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id); static const snort::StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned total, 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 540266e1f..b2e7abec8 100644 --- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc +++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc @@ -63,9 +63,8 @@ static ValidationResult validate_preface(const uint8_t* data, const uint32_t len return V_GOOD; } -StreamSplitter::Status Http2StreamSplitter::data_scan(Http2FlowData* session_data, - const uint8_t* data, uint32_t length, uint32_t* flush_offset, - HttpCommon::SourceId source_id, uint32_t frame_length, uint8_t frame_flags, +StreamSplitter::Status Http2StreamSplitter::data_frame_header_checks(Http2FlowData* session_data, + uint32_t* flush_offset, HttpCommon::SourceId source_id, uint32_t frame_length, uint32_t& data_offset) { Http2Stream* const stream = session_data->find_stream(session_data->current_stream[source_id]); @@ -92,6 +91,7 @@ StreamSplitter::Status Http2StreamSplitter::data_scan(Http2FlowData* session_dat if (frame_length == 0) { *flush_offset = data_offset; + session_data->scan_state[source_id] = SCAN_HEADER; return StreamSplitter::FLUSH; } @@ -104,28 +104,41 @@ StreamSplitter::Status Http2StreamSplitter::data_scan(Http2FlowData* session_dat if (frame_length > MAX_OCTETS) return StreamSplitter::ABORT; - Http2DataCutter* data_cutter = stream->get_data_cutter(source_id); - return data_cutter->scan(data, length, flush_offset, data_offset, frame_length, frame_flags); + return StreamSplitter::SEARCH; } -StreamSplitter::Status Http2StreamSplitter::non_data_scan(Http2FlowData* session_data, - uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id, - uint32_t frame_length, uint8_t type, uint8_t frame_flags, uint32_t& data_offset) +StreamSplitter::Status Http2StreamSplitter::non_data_frame_header_checks( + Http2FlowData* session_data, HttpCommon::SourceId source_id, uint32_t frame_length, + uint8_t type) { // Compute frame section length once per frame - if (session_data->scan_remaining_frame_octets[source_id] == 0) + if (frame_length + FRAME_HEADER_LENGTH > MAX_OCTETS) { - if (frame_length + FRAME_HEADER_LENGTH > MAX_OCTETS) - { - // FIXIT-M long non-data frame needs to be supported - return StreamSplitter::ABORT; - } - - session_data->scan_remaining_frame_octets[source_id] = frame_length; - session_data->total_bytes_in_split[source_id] += FRAME_HEADER_LENGTH + - frame_length; + // FIXIT-M long non-data frame needs to be supported + return StreamSplitter::ABORT; } + + if (type == FT_CONTINUATION and !session_data->continuation_expected[source_id]) + { + // FIXIT-E CONTINUATION frames can also follow PUSH_PROMISE frames, which + // are not currently supported + *session_data->infractions[source_id] += INF_UNEXPECTED_CONTINUATION; + session_data->events[source_id]->create_event( + EVENT_UNEXPECTED_CONTINUATION); + return StreamSplitter::ABORT; + } + + session_data->total_bytes_in_split[source_id] += FRAME_HEADER_LENGTH + + frame_length; + + return StreamSplitter::SEARCH; +} + +StreamSplitter::Status Http2StreamSplitter::non_data_scan(Http2FlowData* session_data, + uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id, uint8_t type, + uint8_t frame_flags, uint32_t& data_offset) +{ // If we don't have the full frame, keep scanning if (length - data_offset < session_data->scan_remaining_frame_octets[source_id]) { @@ -147,25 +160,12 @@ StreamSplitter::Status Http2StreamSplitter::non_data_scan(Http2FlowData* session } break; case FT_CONTINUATION: - if (session_data->continuation_expected[source_id]) - { - if (!(frame_flags & END_HEADERS)) - status = StreamSplitter::SEARCH; - else - { - // continuation frame ending headers - status = StreamSplitter::FLUSH; - session_data->continuation_expected[source_id] = false; - } - } + if (!(frame_flags & END_HEADERS)) + status = StreamSplitter::SEARCH; else { - // FIXIT-E CONTINUATION frames can also follow PUSH_PROMISE frames, which - // are not currently supported - *session_data->infractions[source_id] += INF_UNEXPECTED_CONTINUATION; - session_data->events[source_id]->create_event( - EVENT_UNEXPECTED_CONTINUATION); - status = StreamSplitter::ABORT; + // continuation frame ending headers + session_data->continuation_expected[source_id] = false; } break; default: @@ -176,6 +176,7 @@ StreamSplitter::Status Http2StreamSplitter::non_data_scan(Http2FlowData* session *flush_offset = data_offset; session_data->scan_octets_seen[source_id] = 0; session_data->scan_remaining_frame_octets[source_id] = 0; + session_data->scan_state[source_id] = SCAN_HEADER; return status; } @@ -239,7 +240,7 @@ bool Http2StreamSplitter::read_frame_hdr(Http2FlowData* session_data, const uint StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* session_data, const uint8_t* data, uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id) { - StreamSplitter::Status status = StreamSplitter::FLUSH; + StreamSplitter::Status status = StreamSplitter::SEARCH; if (session_data->preface[source_id]) { // 24-byte preface, not a real frame, no frame header @@ -270,76 +271,143 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio // Need to process multiple frames in a single scan() if a single TCP segment has // 1) multiple header and continuation frames or 2) multiple data frames. - do + while ((status == StreamSplitter::SEARCH) && + ((data_offset < length) or (session_data->scan_state[source_id] == SCAN_EMPTY_DATA))) { - if (session_data->mid_data_frame[source_id]) + switch(session_data->scan_state[source_id]) { - // Continuation of ongoing data frame - Http2Stream* const stream = session_data->find_stream( - session_data->current_stream[source_id]); - Http2DataCutter* data_cutter = stream->get_data_cutter(source_id); - status = data_cutter->scan(data, length, flush_offset, data_offset); - } - else - { - if (!read_frame_hdr(session_data, data, length, source_id, data_offset)) - return StreamSplitter::SEARCH; - - // 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 = 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]; - session_data->current_stream[source_id] = - get_stream_id(session_data->scan_frame_header[source_id]); - - if (session_data->continuation_expected[source_id] && type != FT_CONTINUATION) + case SCAN_HEADER: { - *session_data->infractions[source_id] += INF_MISSING_CONTINUATION; - session_data->events[source_id]->create_event(EVENT_MISSING_CONTINUATION); - return StreamSplitter::ABORT; + if (!read_frame_hdr(session_data, data, length, source_id, data_offset)) + return StreamSplitter::SEARCH; + + // We have the full frame header, compute some variables + 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 uint32_t frame_length = get_frame_length(session_data-> + scan_frame_header[source_id]); + 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]; + session_data->current_stream[source_id] = + get_stream_id(session_data->scan_frame_header[source_id]); + + if (session_data->continuation_expected[source_id] && type != FT_CONTINUATION) + { + *session_data->infractions[source_id] += INF_MISSING_CONTINUATION; + session_data->events[source_id]->create_event(EVENT_MISSING_CONTINUATION); + return StreamSplitter::ABORT; + } + + if (session_data->data_processing[source_id] && + ((type != FT_DATA) || (old_stream != session_data->current_stream[source_id]))) + { + // When there is unflushed data in stream we cannot bypass it to work on some + // other frame. Partial flush gets it out of stream while retaining control of + // message body section sizes. It also avoids extreme delays in inspecting the + // data that could occur if we put this aside indefinitely while processing + // other streams. + const uint32_t next_stream = session_data->current_stream[source_id]; + session_data->current_stream[source_id] = session_data->stream_in_hi = + old_stream; + Http2Stream* const stream = session_data->find_stream( + session_data->current_stream[source_id]); + partial_flush_data(session_data, source_id, flush_offset, data_offset, stream); + + if ((type == FT_HEADERS) and + (session_data->current_stream[source_id]) == next_stream) + { + stream->finish_msg_body(source_id, true, false); + } + session_data->stream_in_hi = NO_STREAM_ID; + return StreamSplitter::FLUSH; + } + + assert(session_data->scan_remaining_frame_octets[source_id] == 0); + session_data->scan_remaining_frame_octets[source_id] = frame_length; + + if (frame_flags & PADDED) + { + if (!(type == FT_DATA || type == FT_HEADERS || type == FT_PUSH_PROMISE)) + { + *session_data->infractions[source_id] += INF_PADDING_ON_INVALID_FRAME; + session_data->events[source_id]->create_event( + EVENT_PADDING_ON_INVALID_FRAME); + return StreamSplitter::ABORT; + } + if (frame_length == 0) + { + *session_data->infractions[source_id] += INF_PADDING_ON_EMPTY_FRAME; + session_data->events[source_id]->create_event( + EVENT_PADDING_ON_EMPTY_FRAME); + return StreamSplitter::ABORT; + } + session_data->scan_state[source_id] = SCAN_PADDING_LENGTH; + } + else if (frame_length == 0) + session_data->scan_state[source_id] = SCAN_EMPTY_DATA; + else + session_data->scan_state[source_id] = SCAN_DATA; + + if (type == FT_DATA) + status = data_frame_header_checks(session_data, flush_offset, source_id, + frame_length, data_offset); + else + status = non_data_frame_header_checks(session_data, source_id, + frame_length, type); + + break; } + case SCAN_PADDING_LENGTH: + assert(session_data->scan_remaining_frame_octets[source_id] > 0); + session_data->padding_length[source_id] = *(data + data_offset); + session_data->scan_remaining_frame_octets[source_id] -= 1; + if (session_data->padding_length[source_id] > + get_frame_length(session_data->scan_frame_header[source_id]) - 1) + { + *session_data->infractions[source_id] += INF_PADDING_LEN; + session_data->events[source_id]->create_event(EVENT_PADDING_LEN); + return StreamSplitter::ABORT; + } + data_offset++; - if (session_data->data_processing[source_id] && - ((type != FT_DATA) || (old_stream != session_data->current_stream[source_id]))) + if (session_data->scan_remaining_frame_octets[source_id] == 0) + { + assert(session_data->padding_length[source_id] == 0); + session_data->scan_state[source_id] = SCAN_EMPTY_DATA; + } + else + session_data->scan_state[source_id] = SCAN_DATA; + break; + case SCAN_DATA: + case SCAN_EMPTY_DATA: { - // When there is unflushed data in stream we cannot bypass it to work on some - // other frame. Partial flush gets it out of stream while retaining control of - // message body section sizes. It also avoids extreme delays in inspecting the - // data that could occur if we put this aside indefinitely while processing - // other streams. - const uint32_t next_stream = session_data->current_stream[source_id]; - session_data->current_stream[source_id] = session_data->stream_in_hi = - old_stream; - Http2Stream* const stream = session_data->find_stream( - session_data->current_stream[source_id]); - partial_flush_data(session_data, source_id, flush_offset, data_offset, stream); - - if ((type == FT_HEADERS) and - (session_data->current_stream[source_id]) == next_stream) + const uint32_t frame_length = get_frame_length(session_data-> + scan_frame_header[source_id]); + const uint8_t type = get_frame_type( + session_data->scan_frame_header[source_id]); + const uint8_t frame_flags = get_frame_flags(session_data-> + scan_frame_header[source_id]); + if (session_data->frame_type[source_id] == FT_DATA) { - stream->finish_msg_body(source_id, true, false); + Http2Stream* const stream = session_data->find_stream( + session_data->current_stream[source_id]); + Http2DataCutter* data_cutter = stream->get_data_cutter(source_id); + status = data_cutter->scan(data, length, flush_offset, data_offset, frame_length, frame_flags); } - session_data->stream_in_hi = NO_STREAM_ID; - return StreamSplitter::FLUSH; + else + status = non_data_scan(session_data, length, flush_offset, source_id, + type, frame_flags, data_offset); + assert(status != StreamSplitter::SEARCH or + session_data->scan_state[source_id] != SCAN_EMPTY_DATA); + break; } - - if (type == FT_DATA) - status = data_scan(session_data, data, length, flush_offset, source_id, - frame_length, frame_flags, data_offset); - else - status = non_data_scan(session_data, length, flush_offset, source_id, - frame_length, type, frame_flags, data_offset); } } - while (status == StreamSplitter::SEARCH && data_offset < length); } return status; @@ -429,7 +497,7 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess session_data->frame_data_offset[source_id] = 0; session_data->remaining_frame_octets[source_id] = 0; - session_data->padding_octets_in_frame[source_id] = 0; + session_data->remaining_padding_octets_in_frame[source_id] = 0; } do @@ -437,22 +505,22 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess uint32_t octets_to_copy; // Read the padding length if necessary - if (session_data->get_padding_len[source_id]) + if (session_data->read_padding_len[source_id]) { - session_data->get_padding_len[source_id] = false; - session_data->padding_octets_in_frame[source_id] = *(data + data_offset); + session_data->read_padding_len[source_id] = false; + session_data->remaining_padding_octets_in_frame[source_id] = *(data + data_offset); data_offset += 1; session_data->remaining_frame_octets[source_id] -= 1; // Subtract the padding and padding length from the frame data size session_data->frame_data_size[source_id] -= - (session_data->padding_octets_in_frame[source_id] + 1); + (session_data->remaining_padding_octets_in_frame[source_id] + 1); } // Copy data into the frame buffer until we run out of data or reach the end of the // current frame's data const uint32_t remaining_frame_payload = session_data->remaining_frame_octets[source_id] - - session_data->padding_octets_in_frame[source_id]; + session_data->remaining_padding_octets_in_frame[source_id]; octets_to_copy = remaining_frame_payload > len - data_offset ? len - data_offset : remaining_frame_payload; if (octets_to_copy > 0) @@ -469,15 +537,18 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess break; // Skip over any padding - uint32_t padding_bytes_to_skip = session_data->padding_octets_in_frame[source_id] > - len - data_offset ? len - data_offset : - session_data->padding_octets_in_frame[source_id]; + uint32_t padding_bytes_to_skip = + session_data->remaining_padding_octets_in_frame[source_id] > len - data_offset ? + len - data_offset : session_data->remaining_padding_octets_in_frame[source_id]; session_data->remaining_frame_octets[source_id] -= padding_bytes_to_skip; + session_data->remaining_padding_octets_in_frame[source_id] -= padding_bytes_to_skip; data_offset += padding_bytes_to_skip; if (data_offset == len) break; + assert(session_data->remaining_padding_octets_in_frame[source_id] == 0); + // Copy headers if (session_data->use_leftover_hdr[source_id]) { @@ -508,22 +579,15 @@ 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); // 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)); + const uint8_t frame_flags = get_frame_flags(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)) - { - session_data->get_padding_len[source_id] = true; - } + if (frame_flags & PADDED) + session_data->read_padding_len[source_id] = true; } while (data_offset < len); } diff --git a/src/service_inspectors/http2_inspect/http2_tables.cc b/src/service_inspectors/http2_inspect/http2_tables.cc index ae1fae5cf..4574cb114 100644 --- a/src/service_inspectors/http2_inspect/http2_tables.cc +++ b/src/service_inspectors/http2_inspect/http2_tables.cc @@ -51,6 +51,8 @@ const RuleMap Http2Module::http2_events[] = { EVENT_PSEUDO_HEADER_IN_TRAILERS, "HTTP/2 pseudo-header in trailers" }, { EVENT_INVALID_PSEUDO_HEADER, "invalid HTTP/2 pseudo-header" }, { EVENT_TRAILERS_NOT_END, "HTTP/2 trailers without END_STREAM bit" }, + { EVENT_PADDING_ON_INVALID_FRAME, "padding flag set on invalid HTTP/2 frame type" }, + { EVENT_PADDING_ON_EMPTY_FRAME, "padding flag set on HTTP/2 frame with zero length" }, { 0, nullptr } };