Skip to content

Commit

Permalink
fix(http1): protect against overflow in chunked decoder
Browse files Browse the repository at this point in the history
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
  • Loading branch information
seanmonstar committed Jul 7, 2021
1 parent 11cb472 commit 1068b99
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 7 deletions.
29 changes: 22 additions & 7 deletions src/proto/h1/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,19 +208,32 @@ impl ChunkedState {
size: &mut u64,
) -> Poll<Result<ChunkedState, io::Error>> {
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;
match byte!(rdr, cx) {
b @ b'0'..=b'9' => {
*size *= radix;
*size += (b - b'0') as u64;
*size = or_overflow!(size.checked_mul(radix));
*size = or_overflow!(size.checked_add((b - b'0') as u64));
}
b @ b'a'..=b'f' => {
*size *= radix;
*size += (b + 10 - b'a') as u64;
*size = or_overflow!(size.checked_mul(radix));
*size = or_overflow!(size.checked_add((b + 10 - b'a') as u64));
}
b @ b'A'..=b'F' => {
*size *= radix;
*size += (b + 10 - b'A') as u64;
*size = or_overflow!(size.checked_mul(radix));
*size = or_overflow!(size.checked_add((b + 10 - b'A') as u64));
}
b'\t' | b' ' => return Poll::Ready(Ok(ChunkedState::SizeLws)),
b';' => return Poll::Ready(Ok(ChunkedState::Extension)),
Expand Down Expand Up @@ -449,7 +462,7 @@ mod tests {

#[tokio::test]
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 {
let mut state = ChunkedState::Size;
Expand Down Expand Up @@ -524,6 +537,8 @@ mod tests {
read_err("1 invalid extension\r\n", InvalidInput).await;
read_err("1 A\r\n", InvalidInput).await;
read_err("1;no CRLF", UnexpectedEof).await;
// Overflow
read_err("f0000000000000003\r\n", InvalidData).await;
}

#[tokio::test]
Expand Down
29 changes: 29 additions & 0 deletions tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,35 @@ fn post_with_chunked_body() {
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]
fn post_with_incomplete_body() {
let _ = pretty_env_logger::try_init();
Expand Down

0 comments on commit 1068b99

Please sign in to comment.