fix(http1): protect against overflow in chunked decoder
The HTTP/1 chunked decoder, when decoding the size of a chunk, could overflow the size if the hex digits were too large. This fixes it by adding an overflow check in the decoder. See GHSA-5h46-h7hh-c6x9
This commit is contained in:
		| @@ -208,19 +208,32 @@ impl ChunkedState { | |||||||
|         size: &mut u64, |         size: &mut u64, | ||||||
|     ) -> Poll<Result<ChunkedState, io::Error>> { |     ) -> Poll<Result<ChunkedState, io::Error>> { | ||||||
|         trace!("Read chunk hex size"); |         trace!("Read chunk hex size"); | ||||||
|  |  | ||||||
|  |         macro_rules! or_overflow { | ||||||
|  |             ($e:expr) => ( | ||||||
|  |                 match $e { | ||||||
|  |                     Some(val) => val, | ||||||
|  |                     None => return Poll::Ready(Err(io::Error::new( | ||||||
|  |                         io::ErrorKind::InvalidData, | ||||||
|  |                         "invalid chunk size: overflow", | ||||||
|  |                     ))), | ||||||
|  |                 } | ||||||
|  |             ) | ||||||
|  |         } | ||||||
|  |  | ||||||
|         let radix = 16; |         let radix = 16; | ||||||
|         match byte!(rdr, cx) { |         match byte!(rdr, cx) { | ||||||
|             b @ b'0'..=b'9' => { |             b @ b'0'..=b'9' => { | ||||||
|                 *size *= radix; |                 *size = or_overflow!(size.checked_mul(radix)); | ||||||
|                 *size += (b - b'0') as u64; |                 *size = or_overflow!(size.checked_add((b - b'0') as u64)); | ||||||
|             } |             } | ||||||
|             b @ b'a'..=b'f' => { |             b @ b'a'..=b'f' => { | ||||||
|                 *size *= radix; |                 *size = or_overflow!(size.checked_mul(radix)); | ||||||
|                 *size += (b + 10 - b'a') as u64; |                 *size = or_overflow!(size.checked_add((b + 10 - b'a') as u64)); | ||||||
|             } |             } | ||||||
|             b @ b'A'..=b'F' => { |             b @ b'A'..=b'F' => { | ||||||
|                 *size *= radix; |                 *size = or_overflow!(size.checked_mul(radix)); | ||||||
|                 *size += (b + 10 - b'A') as u64; |                 *size = or_overflow!(size.checked_add((b + 10 - b'A') as u64)); | ||||||
|             } |             } | ||||||
|             b'\t' | b' ' => return Poll::Ready(Ok(ChunkedState::SizeLws)), |             b'\t' | b' ' => return Poll::Ready(Ok(ChunkedState::SizeLws)), | ||||||
|             b';' => return Poll::Ready(Ok(ChunkedState::Extension)), |             b';' => return Poll::Ready(Ok(ChunkedState::Extension)), | ||||||
| @@ -449,7 +462,7 @@ mod tests { | |||||||
|  |  | ||||||
|     #[tokio::test] |     #[tokio::test] | ||||||
|     async fn test_read_chunk_size() { |     async fn test_read_chunk_size() { | ||||||
|         use std::io::ErrorKind::{InvalidInput, UnexpectedEof}; |         use std::io::ErrorKind::{InvalidData, InvalidInput, UnexpectedEof}; | ||||||
|  |  | ||||||
|         async fn read(s: &str) -> u64 { |         async fn read(s: &str) -> u64 { | ||||||
|             let mut state = ChunkedState::Size; |             let mut state = ChunkedState::Size; | ||||||
| @@ -524,6 +537,8 @@ mod tests { | |||||||
|         read_err("1 invalid extension\r\n", InvalidInput).await; |         read_err("1 invalid extension\r\n", InvalidInput).await; | ||||||
|         read_err("1 A\r\n", InvalidInput).await; |         read_err("1 A\r\n", InvalidInput).await; | ||||||
|         read_err("1;no CRLF", UnexpectedEof).await; |         read_err("1;no CRLF", UnexpectedEof).await; | ||||||
|  |         // Overflow | ||||||
|  |         read_err("f0000000000000003\r\n", InvalidData).await; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     #[tokio::test] |     #[tokio::test] | ||||||
|   | |||||||
| @@ -431,6 +431,35 @@ fn post_with_chunked_body() { | |||||||
|     assert_eq!(server.body(), b"qwert"); |     assert_eq!(server.body(), b"qwert"); | ||||||
| } | } | ||||||
|  |  | ||||||
|  | #[test] | ||||||
|  | fn post_with_chunked_overflow() { | ||||||
|  |     let server = serve(); | ||||||
|  |     let mut req = connect(server.addr()); | ||||||
|  |     req.write_all( | ||||||
|  |         b"\ | ||||||
|  |         POST / HTTP/1.1\r\n\ | ||||||
|  |         Host: example.domain\r\n\ | ||||||
|  |         Transfer-Encoding: chunked\r\n\ | ||||||
|  |         \r\n\ | ||||||
|  |         f0000000000000003\r\n\ | ||||||
|  |         abc\r\n\ | ||||||
|  |         0\r\n\ | ||||||
|  |         \r\n\ | ||||||
|  |         GET /sneaky HTTP/1.1\r\n\ | ||||||
|  |         \r\n\ | ||||||
|  |     ", | ||||||
|  |     ) | ||||||
|  |     .unwrap(); | ||||||
|  |     req.read(&mut [0; 256]).unwrap(); | ||||||
|  |  | ||||||
|  |     let err = server.body_err().to_string(); | ||||||
|  |     assert!( | ||||||
|  |         err.contains("overflow"), | ||||||
|  |         "error should be overflow: {:?}", | ||||||
|  |         err | ||||||
|  |     ); | ||||||
|  | } | ||||||
|  |  | ||||||
| #[test] | #[test] | ||||||
| fn post_with_incomplete_body() { | fn post_with_incomplete_body() { | ||||||
|     let _ = pretty_env_logger::try_init(); |     let _ = pretty_env_logger::try_init(); | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user