From: Philippe Antoine Date: Tue, 25 Jan 2022 08:01:54 +0000 (+0100) Subject: http2: event for variable-length integer overflow X-Git-Tag: suricata-7.0.0-beta1~958 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=df2cbd6517dc751715213a4839581fb98c77af0d;p=thirdparty%2Fsuricata.git http2: event for variable-length integer overflow http2_parse_var_uint can overflow the variable-length integer it is decoding. In this case, it now returns an error of kind LengthValue. The new function http2_parse_headers_blocks, which factorizes the code loop for headers, push promise, and continuation, will check for this specific error, and instead of erroring itself, will return the list of so far parsed headers, plus another one with HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow This status is then checked by process_headers to create an app-layer event. --- diff --git a/rules/http2-events.rules b/rules/http2-events.rules index 8e45fca38d..15b7eed51c 100644 --- a/rules/http2-events.rules +++ b/rules/http2-events.rules @@ -16,3 +16,4 @@ alert http2 any any -> any any (msg:"SURICATA HTTP2 stream identifier reuse"; fl alert http2 any any -> any any (msg:"SURICATA HTTP2 invalid HTTP1 settings during upgrade"; flow:established; app-layer-event:http2.invalid_http1_settings; classtype:protocol-command-decode; sid:2290008; rev:1;) alert http2 any any -> any any (msg:"SURICATA HTTP2 failed decompression"; flow:established; app-layer-event:http2.failed_decompression; classtype:protocol-command-decode; sid:2290009; rev:1;) alert http2 any any -> any any (msg:"SURICATA HTTP2 invalid range header"; flow:established; app-layer-event:http2.invalid_range; classtype:protocol-command-decode; sid:2290010; rev:1;) +alert http2 any any -> any any (msg:"SURICATA HTTP2 variable-length integer overflow"; flow:established; app-layer-event:http2.header_integer_overflow; classtype:protocol-command-decode; sid:2290011; rev:1;) diff --git a/rust/src/http2/http2.rs b/rust/src/http2/http2.rs index 61705c30d0..def71d70e2 100644 --- a/rust/src/http2/http2.rs +++ b/rust/src/http2/http2.rs @@ -366,6 +366,7 @@ pub enum HTTP2Event { InvalidHTTP1Settings, FailedDecompression, InvalidRange, + HeaderIntegerOverflow, } pub struct HTTP2DynTable { @@ -587,6 +588,10 @@ impl HTTP2State { if blocks[i].sizeupdate > sizeup { sizeup = blocks[i].sizeupdate; } + } else if blocks[i].error + == parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow + { + self.set_event(HTTP2Event::HeaderIntegerOverflow); } } if update { @@ -1087,8 +1092,7 @@ pub unsafe extern "C" fn rs_http2_state_tx_free(state: *mut std::os::raw::c_void #[no_mangle] pub unsafe extern "C" fn rs_http2_parse_ts( flow: *const Flow, state: *mut std::os::raw::c_void, _pstate: *mut std::os::raw::c_void, - stream_slice: StreamSlice, - _data: *const std::os::raw::c_void + stream_slice: StreamSlice, _data: *const std::os::raw::c_void, ) -> AppLayerResult { let state = cast_pointer!(state, HTTP2State); let buf = stream_slice.as_slice(); @@ -1101,8 +1105,7 @@ pub unsafe extern "C" fn rs_http2_parse_ts( #[no_mangle] pub unsafe extern "C" fn rs_http2_parse_tc( flow: *const Flow, state: *mut std::os::raw::c_void, _pstate: *mut std::os::raw::c_void, - stream_slice: StreamSlice, - _data: *const std::os::raw::c_void + stream_slice: StreamSlice, _data: *const std::os::raw::c_void, ) -> AppLayerResult { let state = cast_pointer!(state, HTTP2State); let buf = stream_slice.as_slice(); diff --git a/rust/src/http2/parser.rs b/rust/src/http2/parser.rs index a99a5bda15..a4edd84235 100644 --- a/rust/src/http2/parser.rs +++ b/rust/src/http2/parser.rs @@ -365,9 +365,6 @@ fn http2_parse_headers_block_indexed<'a>( } let (i2, indexed) = parser(input)?; let (i3, indexreal) = http2_parse_var_uint(i2, indexed.1 as u64, 0x7F)?; - if indexreal == 0 && indexed.1 == 0x7F { - return Err(Err::Error(make_error(i3, ErrorKind::LengthValue))); - } match http2_frame_header_static(indexreal, dyn_headers) { Some(h) => Ok((i3, h)), _ => Err(Err::Error(make_error(i3, ErrorKind::MapOpt))), @@ -380,9 +377,6 @@ fn http2_parse_headers_block_string(input: &[u8]) -> IResult<&[u8], Vec> { } let (i1, huffslen) = parser(input)?; let (i2, stringlen) = http2_parse_var_uint(i1, huffslen.1 as u64, 0x7F)?; - if stringlen == 0 && huffslen.1 == 0x7F { - return Err(Err::Error(make_error(i2, ErrorKind::LengthValue))); - } let (i3, data) = take(stringlen as usize)(i2)?; if huffslen.0 == 0 { return Ok((i3, data.to_vec())); @@ -437,9 +431,6 @@ fn http2_parse_headers_block_literal_incindex<'a>( } let (i2, indexed) = parser(input)?; let (i3, indexreal) = http2_parse_var_uint(i2, indexed.1 as u64, 0x3F)?; - if indexreal == 0 && indexed.1 == 0x3F { - return Err(Err::Error(make_error(i3, ErrorKind::LengthValue))); - } let r = http2_parse_headers_block_literal_common(i3, indexreal, dyn_headers); match r { Ok((r, head)) => { @@ -491,9 +482,6 @@ fn http2_parse_headers_block_literal_noindex<'a>( } let (i2, indexed) = parser(input)?; let (i3, indexreal) = http2_parse_var_uint(i2, indexed.1 as u64, 0xF)?; - if indexreal == 0 && indexed.1 == 0xF { - return Err(Err::Error(make_error(i3, ErrorKind::LengthValue))); - } let r = http2_parse_headers_block_literal_common(i3, indexreal, dyn_headers); return r; } @@ -509,9 +497,6 @@ fn http2_parse_headers_block_literal_neverindex<'a>( } let (i2, indexed) = parser(input)?; let (i3, indexreal) = http2_parse_var_uint(i2, indexed.1 as u64, 0xF)?; - if indexreal == 0 && indexed.1 == 0xF { - return Err(Err::Error(make_error(i3, ErrorKind::LengthValue))); - } let r = http2_parse_headers_block_literal_common(i3, indexreal, dyn_headers); return r; } @@ -530,12 +515,14 @@ fn http2_parse_var_uint(input: &[u8], value: u64, max: u64) -> IResult<&[u8], u6 for i in 0..varia.len() { varval += ((varia[i] & 0x7F) as u64) << (7 * i); } - if (finalv as u64) << (7 * varia.len()) > u64::MAX - varval { - // this would overflow u64 - return Ok((i3, 0)); + match varval.checked_add((finalv as u64) << (7 * varia.len())) { + None => { + return Err(Err::Error(make_error(i3, ErrorKind::LengthValue))); + } + Some(x) => { + return Ok((i3, x)); + } } - varval += (finalv as u64) << (7 * varia.len()); - return Ok((i3, varval)); } fn http2_parse_headers_block_dynamic_size<'a>( @@ -549,17 +536,6 @@ fn http2_parse_headers_block_dynamic_size<'a>( } let (i2, maxsize) = parser(input)?; let (i3, maxsize2) = http2_parse_var_uint(i2, maxsize.1 as u64, 0x1F)?; - if maxsize2 == 0 && maxsize.1 == 0x1F { - return Ok(( - i3, - HTTP2FrameHeaderBlock { - name: Vec::new(), - value: Vec::new(), - error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow, - sizeupdate: 0, - }, - )); - } if (maxsize2 as usize) < dyn_headers.max_size { //dyn_headers.max_size is updated later with all headers //may evict entries @@ -612,15 +588,11 @@ pub const HTTP2_FLAG_HEADER_END_HEADERS: u8 = 0x4; const HTTP2_FLAG_HEADER_PADDED: u8 = 0x8; const HTTP2_FLAG_HEADER_PRIORITY: u8 = 0x20; -pub fn http2_parse_frame_headers<'a>( - input: &'a [u8], flags: u8, dyn_headers: &mut HTTP2DynTable, -) -> IResult<&'a [u8], HTTP2FrameHeaders> { - let (i2, padlength) = cond(flags & HTTP2_FLAG_HEADER_PADDED != 0, be_u8)(input)?; - let (mut i3, priority) = cond( - flags & HTTP2_FLAG_HEADER_PRIORITY != 0, - http2_parse_headers_priority, - )(i2)?; +fn http2_parse_headers_blocks<'a>( + input: &'a [u8], dyn_headers: &mut HTTP2DynTable, +) -> IResult<&'a [u8], Vec> { let mut blocks = Vec::new(); + let mut i3 = input; while i3.len() > 0 { match http2_parse_headers_block(i3, dyn_headers) { Ok((rem, b)) => { @@ -632,11 +604,35 @@ pub fn http2_parse_frame_headers<'a>( } i3 = rem; } + Err(Err::Error(ref err)) => { + // if we error from http2_parse_var_uint, we keep the first parsed headers + if err.code == ErrorKind::LengthValue { + blocks.push(HTTP2FrameHeaderBlock { + name: Vec::new(), + value: Vec::new(), + error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow, + sizeupdate: 0, + }); + break; + } + } Err(x) => { return Err(x); } } } + return Ok((i3, blocks)); +} + +pub fn http2_parse_frame_headers<'a>( + input: &'a [u8], flags: u8, dyn_headers: &mut HTTP2DynTable, +) -> IResult<&'a [u8], HTTP2FrameHeaders> { + let (i2, padlength) = cond(flags & HTTP2_FLAG_HEADER_PADDED != 0, be_u8)(input)?; + let (i3, priority) = cond( + flags & HTTP2_FLAG_HEADER_PRIORITY != 0, + http2_parse_headers_priority, + )(i2)?; + let (i3, blocks) = http2_parse_headers_blocks(i3, dyn_headers)?; return Ok(( i3, HTTP2FrameHeaders { @@ -659,24 +655,8 @@ pub fn http2_parse_frame_push_promise<'a>( input: &'a [u8], flags: u8, dyn_headers: &mut HTTP2DynTable, ) -> IResult<&'a [u8], HTTP2FramePushPromise> { let (i2, padlength) = cond(flags & HTTP2_FLAG_HEADER_PADDED != 0, be_u8)(input)?; - let (mut i3, stream_id) = bits(tuple((take_bits(1u8), take_bits(31u32))))(i2)?; - let mut blocks = Vec::new(); - while i3.len() > 0 { - match http2_parse_headers_block(i3, dyn_headers) { - Ok((rem, b)) => { - blocks.push(b); - debug_validate_bug_on!(i3.len() == rem.len()); - if i3.len() == rem.len() { - //infinite loop - return Err(Err::Error(make_error(input, ErrorKind::Eof))); - } - i3 = rem; - } - Err(x) => { - return Err(x); - } - } - } + let (i3, stream_id) = bits(tuple((take_bits(1u8), take_bits(31u32))))(i2)?; + let (i3, blocks) = http2_parse_headers_blocks(i3, dyn_headers)?; return Ok(( i3, HTTP2FramePushPromise { @@ -696,24 +676,7 @@ pub struct HTTP2FrameContinuation { pub fn http2_parse_frame_continuation<'a>( input: &'a [u8], dyn_headers: &mut HTTP2DynTable, ) -> IResult<&'a [u8], HTTP2FrameContinuation> { - let mut i3 = input; - let mut blocks = Vec::new(); - while i3.len() > 0 { - match http2_parse_headers_block(i3, dyn_headers) { - Ok((rem, b)) => { - blocks.push(b); - debug_validate_bug_on!(i3.len() == rem.len()); - if i3.len() == rem.len() { - //infinite loop - return Err(Err::Error(make_error(input, ErrorKind::Eof))); - } - i3 = rem; - } - Err(x) => { - return Err(x); - } - } - } + let (i3, blocks) = http2_parse_headers_blocks(input, dyn_headers)?; return Ok((i3, HTTP2FrameContinuation { blocks })); }