Skip to content

Commit

Permalink
fix(header): fix panic when parsing header names larger than 64kb
Browse files Browse the repository at this point in the history
  • Loading branch information
seanmonstar committed Nov 6, 2018
1 parent 976a77a commit 9245e94
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 3 deletions.
16 changes: 13 additions & 3 deletions src/proto/h1/role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl Http1Transaction for Server {
Version::HTTP_10
};

record_header_indices(bytes, &req.headers, &mut headers_indices);
record_header_indices(bytes, &req.headers, &mut headers_indices)?;
headers_len = req.headers.len();
//(len, subject, version, headers_len)
}
Expand Down Expand Up @@ -590,7 +590,7 @@ impl Http1Transaction for Client {
} else {
Version::HTTP_10
};
record_header_indices(bytes, &res.headers, &mut headers_indices);
record_header_indices(bytes, &res.headers, &mut headers_indices)?;
let headers_len = res.headers.len();
(len, status, version, headers_len)
},
Expand Down Expand Up @@ -918,7 +918,11 @@ struct HeaderIndices {
value: (usize, usize),
}

fn record_header_indices(bytes: &[u8], headers: &[httparse::Header], indices: &mut [HeaderIndices]) {
fn record_header_indices(
bytes: &[u8],
headers: &[httparse::Header],
indices: &mut [HeaderIndices]
) -> Result<(), ::error::Parse> {
let bytes_ptr = bytes.as_ptr() as usize;

// FIXME: This should be a single plain `for` loop.
Expand All @@ -945,6 +949,10 @@ fn record_header_indices(bytes: &[u8], headers: &[httparse::Header], indices: &m
cfg(all(target_arch = "arm", target_feature = "v7", target_feature = "neon"))
for (header, indices) in (headers.iter().zip(indices.iter_mut())) {
{
if header.name.len() >= (1 << 16) {
debug!("header name larger than 64kb: {:?}", header.name);
return Err(::error::Parse::TooLarge);
}
let name_start = header.name.as_ptr() as usize - bytes_ptr;
let name_end = name_start + header.name.len();
indices.name = (name_start, name_end);
Expand All @@ -956,6 +964,8 @@ fn record_header_indices(bytes: &[u8], headers: &[httparse::Header], indices: &m
}
}
}

Ok(())
}

// Write header names as title case. The header name is assumed to be ASCII,
Expand Down
18 changes: 18 additions & 0 deletions tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,24 @@ fn returning_1xx_response_is_error() {
rt.block_on(fut).expect_err("1xx status code should error");
}

#[test]
fn header_name_too_long() {
let server = serve();

let mut req = connect(server.addr());
let mut write = Vec::with_capacity(1024 * 66);
write.extend_from_slice(b"GET / HTTP/1.1\r\n");
for _ in 0..(1024 * 65) {
write.push(b'x');
}
write.extend_from_slice(b": foo\r\n\r\n");
req.write_all(&write).unwrap();

let mut buf = [0; 1024];
let n = req.read(&mut buf).unwrap();
assert!(s(&buf[..n]).starts_with("HTTP/1.1 431 Request Header Fields Too Large\r\n"));
}

#[test]
fn upgrades() {
use tokio_io::io::{read_to_end, write_all};
Expand Down

0 comments on commit 9245e94

Please sign in to comment.