Skip to content

fix: validate transfer encoding syntax #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mcollina wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: validate transfer encoding syntax #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions parser/fuzz/tests/conformance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ fn rejects_transfer_encoding_not_ending_in_chunked() {
}
}

#[test]
fn rejects_invalid_transfer_encoding_syntax() {
for input in [
&b"POST / HTTP/1.1\r\nTransfer-Encoding: bad@coding, chunked\r\n\r\n0\r\n\r\n"[..],
&b"POST / HTTP/1.1\r\nTransfer-Encoding: gzip,, chunked\r\n\r\n0\r\n\r\n"[..],
&b"POST / HTTP/1.1\r\nTransfer-Encoding: gzip; level=bad@value, chunked\r\n\r\n0\r\n\r\n"[..],
&b"POST / HTTP/1.1\r\nTransfer-Encoding: chunked; level=9\r\n\r\n0\r\n\r\n"[..],
] {
assert_ne!(error_code(input, true), ERROR_NONE, "invalid TE syntax must be rejected: {input:?}");
}
}

// --- Well-formed messages must still be accepted. ---

#[test]
Expand Down
125 changes: 125 additions & 0 deletions parser/src/matchers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,131 @@ pub fn validate_quoted_string(data: &[u8], start: usize, end: usize) -> bool {
true
}

/// Validates a Transfer-Encoding field value and reports whether its final
/// coding is `chunked`. List separators inside quoted parameter values are not
/// treated as coding separators.
#[inline(always)]
pub fn validate_transfer_encoding(data: &[u8], start: usize, end: usize) -> Option<bool> {
let mut i = start;
let mut has_chunked = false;

while i < end {
while i < end && is_ws(data[i]) {
i += 1;
}

let coding_start = i;
while i < end && TOKEN_TABLE[data[i] as usize] {
i += 1;
}
if coding_start == i {
return None;
}

let is_chunked = data[coding_start..i].eq_ignore_ascii_case(b"chunked");
let mut has_parameters = false;

while i < end && is_ws(data[i]) {
i += 1;
}

while i < end && data[i] == b';' {
has_parameters = true;
i += 1;

while i < end && is_ws(data[i]) {
i += 1;
}

let parameter_name_start = i;
while i < end && TOKEN_TABLE[data[i] as usize] {
i += 1;
}
if parameter_name_start == i {
return None;
}

while i < end && is_ws(data[i]) {
i += 1;
}
if i == end || data[i] != b'=' {
return None;
}
i += 1;

while i < end && is_ws(data[i]) {
i += 1;
}
if i == end {
return None;
}

if data[i] == b'"' {
i += 1;
let value_start = i;
loop {
if i == end {
return None;
}

match data[i] {
b'"' => break,
b'\\' => {
i += 1;
if i == end {
return None;
}
}
_ => {}
}
i += 1;
}

if !validate_quoted_string(data, value_start, i) {
return None;
}
i += 1;
} else {
let value_start = i;
while i < end && TOKEN_TABLE[data[i] as usize] {
i += 1;
}
if value_start == i {
return None;
}
}

while i < end && is_ws(data[i]) {
i += 1;
}
}

if is_chunked {
if has_chunked || has_parameters {
return None;
}
has_chunked = true;
}

if i == end {
return Some(has_chunked);
}
if data[i] != b',' || has_chunked {
return None;
}

i += 1;
while i < end && is_ws(data[i]) {
i += 1;
}
if i == end {
return None;
}
}

None
}

#[inline(always)]
pub fn validate_url(data: &[u8], start: usize, end: usize) -> bool {
if start == end {
Expand Down
85 changes: 19 additions & 66 deletions parser/src/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,75 +584,28 @@ impl Parser {
fail!(UNEXPECTED_CHARACTER, "Expected Transfer-Encoding header value");
}

self.has_transfer_encoding = true;

if &data[header_value_start..header_value_end] == b"chunked" {
// If this is true, it means the Transfer-Encoding header was specified more
// than once. This is the second repetition and therefore, the previous one is
// no longer the last one, making it invalid.
if self.has_chunked_transfer_encoding {
fail!(
INVALID_TRANSFER_ENCODING,
"The value \"chunked\" in the Transfer-Encoding header must be the last provided and can \
be provided only once"
);
}

self.has_chunked_transfer_encoding = true;
} else {
let mut token_start = header_value_start;
loop {
while token_start < header_value_end && is_ws(data[token_start]) {
token_start += 1;
}

if token_start == header_value_end {
break;
}

let token_end_raw = match find_char(data, token_start, header_value_end, b',') {
Some(comma) => comma,
None => header_value_end,
};
let mut token_end = token_end_raw;

if !strip_ows_fast(data, &mut token_start, &mut token_end, false) {
fail!(UNEXPECTED_CHARACTER, "Expected Transfer-Encoding header value");
}

self.has_transfer_encoding = true;

if let case_insensitive_string!("chunked") = data[token_start..token_end] {
// If this is true, it means the Transfer-Encoding header was specified more
// than once. This is the second repetition and therefore, the previous one is
// no longer the last one, making it invalid.
if self.has_chunked_transfer_encoding {
fail!(
INVALID_TRANSFER_ENCODING,
"The value \"chunked\" in the Transfer-Encoding header must be the last provided and \
can be provided only once"
);
}

self.has_chunked_transfer_encoding = true;
} else {
if self.has_chunked_transfer_encoding {
// Any other value when chunked was already specified is invalid as the previous
// chunked would not be the last one anymore
fail!(
INVALID_TRANSFER_ENCODING,
"The value \"chunked\" in the Transfer-Encoding header must be the last provided"
);
}
let header_has_chunked =
match validate_transfer_encoding(data, header_value_start, header_value_end) {
Some(has_chunked) => has_chunked,
None => {
fail!(INVALID_TRANSFER_ENCODING, "Invalid Transfer-Encoding header value");
}
};

if token_end_raw == header_value_end {
break;
} else {
token_start = token_end_raw + 1;
}
}
// A later field value would make a previously observed chunked coding
// non-final.
if self.has_chunked_transfer_encoding {
fail!(
INVALID_TRANSFER_ENCODING,
"The value \"chunked\" in the Transfer-Encoding header must be the last provided and can be \
provided only once"
);
}

// Do not update framing state until every coding and parameter in this field
// value has been validated.
self.has_transfer_encoding = true;
self.has_chunked_transfer_encoding = header_has_chunked;
}
// RFC 9112 section 9.6
(10, case_insensitive_string!("connection")) => {
Expand Down
81 changes: 81 additions & 0 deletions parser/tests/compliance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,87 @@ fn compliance_chunked_must_be_final() {
assert_error(&parser);
}

// Transfer-coding names use HTTP token syntax.
#[test]
fn compliance_transfer_encoding_rejects_invalid_coding_name() {
let mut parser = response_parser();
let message = wire("HTTP/1.1 200 OK\r\nTransfer-Encoding: bad@coding, chunked\r\n\r\n0\r\n\r\n");

parse(&mut parser, &message);

assert_error(&parser);
assert!(!parser.has_transfer_encoding);
assert!(!parser.has_chunked_transfer_encoding);
}

// Transfer-Encoding list members cannot be empty.
#[test]
fn compliance_transfer_encoding_rejects_empty_list_members() {
for value in [", chunked", "gzip,, chunked", "gzip, chunked,"] {
let mut parser = response_parser();
let message = wire(&format!(
"HTTP/1.1 200 OK\r\nTransfer-Encoding: {value}\r\n\r\n0\r\n\r\n"
));

parse(&mut parser, &message);

assert_error(&parser);
}
}

// Transfer parameters require token names and token or quoted-string values.
#[test]
fn compliance_transfer_encoding_rejects_invalid_parameters() {
for value in [
"gzip; level, chunked",
"gzip; =9, chunked",
"gzip; level=, chunked",
"gzip; level=bad@value, chunked",
"gzip; level=\"unterminated, chunked",
"gzip; level=\"bad\u{1}\", chunked",
"chunked; level=9",
] {
let mut parser = response_parser();
let message = wire(&format!(
"HTTP/1.1 200 OK\r\nTransfer-Encoding: {value}\r\n\r\n0\r\n\r\n"
));

parse(&mut parser, &message);

assert_error(&parser);
}
}

// Valid non-chunked parameters are preserved, including quoted commas, before
// final chunked.
#[test]
fn compliance_transfer_encoding_accepts_parameters_before_chunked() {
let mut parser = response_parser();
let message = wire("HTTP/1.1 200 OK\r\nTransfer-Encoding: gzip; level = 9; note=\"a,b\", ChUnKeD\r\n\r\n0\r\n\r\n");

parse(&mut parser, &message);

assert_ok(&parser);
assert!(parser.has_transfer_encoding);
assert!(parser.has_chunked_transfer_encoding);
}

// Chunked can occur only once, even across multiple field lines.
#[test]
fn compliance_transfer_encoding_rejects_duplicate_chunked() {
for headers in [
"Transfer-Encoding: gzip, chunked, chunked\r\n",
"Transfer-Encoding: gzip, chunked\r\nTransfer-Encoding: chunked\r\n",
] {
let mut parser = response_parser();
let message = wire(&format!("HTTP/1.1 200 OK\r\n{headers}\r\n0\r\n\r\n"));

parse(&mut parser, &message);

assert_error(&parser);
}
}

// Content-Length cannot be combined with Transfer-Encoding.
#[test]
fn compliance_content_length_transfer_encoding_conflict() {
Expand Down
Loading
Loading