From: Tom Peters (thopeter) Date: Mon, 5 Mar 2018 16:20:20 +0000 (-0500) Subject: Merge pull request #1123 in SNORT/snort3 from nhttp95 to master X-Git-Tag: 3.0.0-244~14 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=11491bfa4e153518a91e80f92b1673fd6a1f32b2;p=thirdparty%2Fsnort3.git Merge pull request #1123 in SNORT/snort3 from nhttp95 to master Squashed commit of the following: commit 83b52927abd49f59d40d54966f789960fb7b4ec1 Author: Tom Peters Date: Sat Mar 3 13:08:08 2018 -0500 http_inspect: white space before chunk length --- diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc index d8c81730e..85c49eddf 100644 --- a/src/service_inspectors/http_inspect/http_cutter.cc +++ b/src/service_inspectors/http_inspect/http_cutter.cc @@ -327,9 +327,26 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length, events->create_event(EVENT_CHUNK_BAD_SEP); break; } - curr_state = CHUNK_ZEROS; + curr_state = CHUNK_LEADING_WS; k--; // Reprocess this octet in the next state break; + case CHUNK_LEADING_WS: + // Looking for whitespace before the chunk size + if (is_sp_tab[buffer[k]]) + { + *infractions += INF_CHUNK_LEADING_WS; + events->create_event(EVENT_CHUNK_WHITESPACE); + num_leading_ws++; + if (num_leading_ws == 5) + { + events->create_event(EVENT_BROKEN_CHUNK); + curr_state = CHUNK_BAD; + } + break; + } + curr_state = CHUNK_ZEROS; + k--; + break; case CHUNK_ZEROS: // Looking for leading zeros in the chunk size. if (buffer[k] == '0') @@ -362,7 +379,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length, { *infractions += INF_CHUNK_WHITESPACE; events->create_event(EVENT_CHUNK_WHITESPACE); - curr_state = CHUNK_WHITESPACE; + curr_state = CHUNK_TRAILING_WS; } else if (buffer[k] == ';') { @@ -389,7 +406,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length, } } break; - case CHUNK_WHITESPACE: + case CHUNK_TRAILING_WS: // Skipping over improper whitespace following the chunk size if (buffer[k] == '\r') { @@ -507,6 +524,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length, case CHUNK_DCRLF2: // The LF from the end-of-chunk CRLF should be here num_good_chunks++; + num_leading_ws = 0; num_zeros = 0; expected = 0; digits_seen = 0; diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h index 656cf0302..68ffd6fd5 100644 --- a/src/service_inspectors/http_inspect/http_cutter.h +++ b/src/service_inspectors/http_inspect/http_cutter.h @@ -125,6 +125,7 @@ private: uint32_t data_seen = 0; HttpEnums::ChunkState curr_state = HttpEnums::CHUNK_NEWLINES; uint32_t expected = 0; + uint32_t num_leading_ws = 0; uint32_t num_zeros = 0; uint32_t digits_seen = 0; bool new_section = false; diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h index 562178abc..0bd4c98ae 100644 --- a/src/service_inspectors/http_inspect/http_enum.h +++ b/src/service_inspectors/http_inspect/http_enum.h @@ -71,8 +71,8 @@ enum ScanResult { SCAN_NOTFOUND, SCAN_FOUND, SCAN_FOUND_PIECE, SCAN_DISCARD, SCA SCAN_ABORT, SCAN_END }; // State machine for chunk parsing -enum ChunkState { CHUNK_NEWLINES, CHUNK_ZEROS, CHUNK_NUMBER, CHUNK_WHITESPACE, CHUNK_OPTIONS, - CHUNK_HCRLF, CHUNK_DATA, CHUNK_DCRLF1, CHUNK_DCRLF2, CHUNK_BAD }; +enum ChunkState { CHUNK_NEWLINES, CHUNK_ZEROS, CHUNK_LEADING_WS, CHUNK_NUMBER, CHUNK_TRAILING_WS, + CHUNK_OPTIONS, CHUNK_HCRLF, CHUNK_DATA, CHUNK_DCRLF1, CHUNK_DCRLF2, CHUNK_BAD }; // List of possible HTTP versions. enum VersionId { VERS__NO_SOURCE=-16, VERS__NOT_COMPUTE=-14, VERS__PROBLEMATIC=-12, @@ -231,6 +231,7 @@ enum Infraction INF_CONTENT_ENCODING_CHUNKED, INF_206_WITHOUT_RANGE, INF_VERSION_NOT_UPPERCASE, + INF_CHUNK_LEADING_WS, INF__MAX_VALUE }; 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 3df342e0f..b2e58d4fd 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc @@ -45,7 +45,9 @@ void HttpStreamSplitter::chunk_spray(HttpFlowData* session_data, uint8_t* buffer switch (curr_state) { case CHUNK_NEWLINES: - if (!is_cr_lf[data[k]]) + case CHUNK_LEADING_WS: + // Cases are combined in reassemble(). CHUNK_LEADING_WS here to avoid compiler warning. + if (!is_sp_tab_cr_lf[data[k]]) { curr_state = CHUNK_NUMBER; k--; @@ -64,13 +66,13 @@ void HttpStreamSplitter::chunk_spray(HttpFlowData* session_data, uint8_t* buffer else if (data[k] == ';') curr_state = CHUNK_OPTIONS; else if (is_sp_tab[data[k]]) - curr_state = CHUNK_WHITESPACE; + curr_state = CHUNK_TRAILING_WS; else expected = expected * 16 + as_hex[data[k]]; break; + case CHUNK_TRAILING_WS: case CHUNK_OPTIONS: - case CHUNK_WHITESPACE: - // No practical difference between white space and options in reassemble() + // No practical difference between trailing white space and options in reassemble() if (data[k] == '\r') curr_state = CHUNK_HCRLF; else if (data[k] == '\n') diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc index b9ca31649..9afbfb356 100644 --- a/src/service_inspectors/http_inspect/http_tables.cc +++ b/src/service_inspectors/http_inspect/http_tables.cc @@ -343,7 +343,7 @@ const RuleMap HttpModule::http_events[] = { EVENT_URI_BAD_FORMAT, "URI badly formatted" }, { EVENT_UNKNOWN_PERCENT, "unrecognized type of percent encoding in URI" }, { EVENT_BROKEN_CHUNK, "HTTP chunk misformatted" }, - { EVENT_CHUNK_WHITESPACE, "white space following chunk length" }, + { EVENT_CHUNK_WHITESPACE, "white space adjacent to chunk length" }, { EVENT_HEAD_NAME_WHITESPACE, "white space within header name" }, { EVENT_GZIP_OVERRUN, "excessive gzip compression" }, { EVENT_GZIP_FAILURE, "gzip decompression failed" },