Skip to content

Normalize bare carriage returns in header line endings #13691

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
RajaMuhammadAwais wants to merge 1 commit into
base: master
Choose a base branch
from
+63 −8
Open

Normalize bare carriage returns in header line endings #13691

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
14 changes: 11 additions & 3 deletions src/proxy/hdrs/MIME.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2559,15 +2559,23 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char
}
field_name.rtrim_if(&ParseRules::is_ws);
raw_print_field = false;
} else if (parsed.suffix(2) != "\r\n" || (parsed.size() > 2 && parsed[parsed.size() - 3] == '\r')) {
// Do not preserve malformed line endings when forwarding the field.
raw_print_field = false;
}

// find value first
field_value.ltrim_if(&ParseRules::is_ws);
field_value.rtrim_if(&ParseRules::is_wslfcr);

if (raw_print_field) {
// Raw printing copies the original input bytes instead of re-serializing, so it would
// replay this line ending verbatim. Everything between the end of the trimmed value and
// the end of the line must be optional whitespace followed by exactly one CRLF; a bare
// CR in there is malformed (RFC 9112 section 2.2) and must not be forwarded.
TextView tail{field_value.data() + field_value.size(), parsed.data() + parsed.size()};
if (tail.suffix(2) != "\r\n" || tail.remove_suffix(2).find_first_of("\r\n") != TextView::npos) {
raw_print_field = false;
}
}

// Make sure the name + value is not longer than configured max_hdr_field_size
if (field_name.size() + field_value.size() > max_hdr_field_size) {
return ParseResult::ERROR;
Expand Down
57 changes: 52 additions & 5 deletions src/proxy/hdrs/unit_tests/test_Hdrs.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3175,9 +3175,28 @@ TEST_CASE("HTTP parser tolerates high-bit bytes without UB", "[proxy][hdrtest]")
req_hdr.destroy();
}

TEST_CASE("HTTP parser normalizes repeated carriage returns in header line endings", "[proxy][hdrtest]")
TEST_CASE("HTTP request parser normalizes bare carriage returns in header line endings", "[proxy][hdrtest]")
{
constexpr std::string_view message = "GET / HTTP/1.1\r\nHost: example.com\r\nExtra-CRs: \r\r\r\r\n\r\n"sv;
struct Test {
std::string_view raw_value;
std::string_view normalized_field;
};

// The four-CR case from #13595 produces the maximum raw-print pad and is rejected by the
// existing pad-size limit alone. These smaller pads exercise the parser check independently.
static const std::vector<Test> tests = {
{"\r \r\n"sv, "Extra-CRs: \r\n"sv },
{"\r\r \r\n"sv, "Extra-CRs: \r\n"sv },
{"bar\r \r\n"sv, "Extra-CRs: bar\r\n"sv},
{"\r\t\r\n"sv, "Extra-CRs: \r\n"sv },
};

auto test = GENERATE(from_range(tests));
CAPTURE(test.raw_value);

std::string message = "GET / HTTP/1.1\r\nHost: example.com\r\nExtra-CRs: ";
message += test.raw_value;
message += "\r\n";

HTTPParser parser;
http_parser_init(&parser);
Expand All @@ -3186,7 +3205,7 @@ TEST_CASE("HTTP parser normalizes repeated carriage returns in header line endin
HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64);
req_hdr.create(HTTPType::REQUEST, HTTP_1_1, heap);

auto start = message.data();
const char *start = message.data();
REQUIRE(req_hdr.parse_req(&parser, &start, message.data() + message.size(), true) == ParseResult::DONE);

std::string serialized(static_cast<size_t>(req_hdr.length_get()), '\0');
Expand All @@ -3195,12 +3214,40 @@ TEST_CASE("HTTP parser normalizes repeated carriage returns in header line endin
req_hdr.print(serialized.data(), static_cast<int>(serialized.size()), &index, &offset);
serialized.resize(static_cast<size_t>(index));

CHECK(serialized.find("Extra-CRs: \r\n") != std::string::npos);
CHECK(serialized.find("Extra-CRs: \r\r") == std::string::npos);
CHECK(serialized.find(test.normalized_field) != std::string::npos);
std::string raw_field = "Extra-CRs: ";
raw_field += test.raw_value;
CHECK(serialized.find(raw_field) == std::string::npos);

req_hdr.destroy();
}

TEST_CASE("HTTP response parser normalizes bare carriage returns in header line endings", "[proxy][hdrtest]")
{
constexpr std::string_view message = "HTTP/1.1 200 OK\r\nExtra-CRs: bar\r \r\n\r\n"sv;

HTTPParser parser;
http_parser_init(&parser);

HTTPHdr resp_hdr;
HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64);
resp_hdr.create(HTTPType::RESPONSE, HTTP_1_1, heap);

const char *start = message.data();
REQUIRE(resp_hdr.parse_resp(&parser, &start, message.data() + message.size(), true) == ParseResult::DONE);

std::string serialized(static_cast<size_t>(resp_hdr.length_get()), '\0');
int index = 0;
int offset = 0;
resp_hdr.print(serialized.data(), static_cast<int>(serialized.size()), &index, &offset);
serialized.resize(static_cast<size_t>(index));

CHECK(serialized.find("Extra-CRs: bar\r\n") != std::string::npos);
CHECK(serialized.find("Extra-CRs: bar\r \r\n") == std::string::npos);

resp_hdr.destroy();
}

TEST_CASE("HTTP response parser tolerates high-bit bytes without UB", "[proxy][hdrtest]")
{
struct Test {
Expand Down