diff --git a/actix-http/src/h1/decoder.rs b/actix-http/src/h1/decoder.rs index 8e891dc5c..e83cf8f46 100644 --- a/actix-http/src/h1/decoder.rs +++ b/actix-http/src/h1/decoder.rs @@ -67,6 +67,7 @@ pub(crate) trait MessageType: Sized { let mut has_upgrade_websocket = false; let mut expect = false; let mut chunked = false; + let mut seen_te = false; let mut content_length = None; { @@ -85,8 +86,17 @@ pub(crate) trait MessageType: Sized { }; match name { - header::CONTENT_LENGTH => { - if let Ok(s) = value.to_str() { + header::CONTENT_LENGTH if content_length.is_some() => { + debug!("multiple Content-Length"); + return Err(ParseError::Header); + } + + header::CONTENT_LENGTH => match value.to_str() { + Ok(s) if s.trim().starts_with("+") => { + debug!("illegal Content-Length: {:?}", s); + return Err(ParseError::Header); + } + Ok(s) => { if let Ok(len) = s.parse::() { if len != 0 { content_length = Some(len); @@ -95,15 +105,31 @@ pub(crate) trait MessageType: Sized { debug!("illegal Content-Length: {:?}", s); return Err(ParseError::Header); } - } else { + } + Err(_) => { debug!("illegal Content-Length: {:?}", value); return Err(ParseError::Header); } - } + }, + // transfer-encoding + header::TRANSFER_ENCODING if seen_te => { + debug!("multiple Transfer-Encoding not allowed"); + return Err(ParseError::Header); + } + header::TRANSFER_ENCODING => { + seen_te = true; + if let Ok(s) = value.to_str().map(|s| s.trim()) { - chunked = s.eq_ignore_ascii_case("chunked"); + if s.eq_ignore_ascii_case("chunked") { + chunked = true; + } else if s.eq_ignore_ascii_case("identity") { + // allow silently since multiple TE headers are already checked + } else { + debug!("illegal Transfer-Encoding: {:?}", s); + return Err(ParseError::Header); + } } else { return Err(ParseError::Header); } @@ -510,19 +536,11 @@ impl ChunkedState { size: &mut u64, ) -> Poll> { let radix = 16; - match byte!(rdr) { - b @ b'0'..=b'9' => { - *size *= radix; - *size += u64::from(b - b'0'); - } - b @ b'a'..=b'f' => { - *size *= radix; - *size += u64::from(b + 10 - b'a'); - } - b @ b'A'..=b'F' => { - *size *= radix; - *size += u64::from(b + 10 - b'A'); - } + + let rem = match byte!(rdr) { + b @ b'0'..=b'9' => b - b'0', + b @ b'a'..=b'f' => b + 10 - b'a', + b @ b'A'..=b'F' => b + 10 - b'A', b'\t' | b' ' => return Poll::Ready(Ok(ChunkedState::SizeLws)), b';' => return Poll::Ready(Ok(ChunkedState::Extension)), b'\r' => return Poll::Ready(Ok(ChunkedState::SizeLf)), @@ -532,8 +550,23 @@ impl ChunkedState { "Invalid chunk size line: Invalid Size", ))); } + }; + + match size.checked_mul(radix) { + Some(n) => { + *size = n as u64; + *size += rem as u64; + + Poll::Ready(Ok(ChunkedState::Size)) + } + None => { + debug!("chunk size would overflow"); + Poll::Ready(Err(io::Error::new( + io::ErrorKind::InvalidInput, + "Invalid chunk size line: Invalid Size", + ))) + } } - Poll::Ready(Ok(ChunkedState::Size)) } fn read_size_lws(rdr: &mut BytesMut) -> Poll> { @@ -552,6 +585,11 @@ impl ChunkedState { fn read_extension(rdr: &mut BytesMut) -> Poll> { match byte!(rdr) { b'\r' => Poll::Ready(Ok(ChunkedState::SizeLf)), + // strictly 0x20 (space) should be disallowed but we don't parse quoted strings here + 0x00..=0x08 | 0x0a..=0x1f | 0x7f => Poll::Ready(Err(io::Error::new( + io::ErrorKind::InvalidInput, + "Invalid character in chunk extension", + ))), _ => Poll::Ready(Ok(ChunkedState::Extension)), // no supported extensions } } @@ -977,13 +1015,7 @@ mod tests { "GET /test HTTP/1.1\r\n\ transfer-encoding: chnked\r\n\r\n", ); - let req = parse_ready!(&mut buf); - - if let Ok(val) = req.chunked() { - assert!(!val); - } else { - unreachable!("Error"); - } + expect_parse_err!(&mut buf); } #[test] diff --git a/actix-http/src/h1/encoder.rs b/actix-http/src/h1/encoder.rs index b7648eff1..1e30652dd 100644 --- a/actix-http/src/h1/encoder.rs +++ b/actix-http/src/h1/encoder.rs @@ -80,6 +80,7 @@ pub(crate) trait MessageType: Sized { match length { BodySize::Stream => { if chunked { + skip_len = true; if camel_case { dst.put_slice(b"\r\nTransfer-Encoding: chunked\r\n") } else {