Add new validation policy rejecting chunked requests framed with anything other than CRLFCRLF
According to the [RFC](https://datatracker.ietf.org/doc/html/rfc9112#name-chunked-transfer-coding), the ABNF for chunked coding states that both `last-chunk` _and_ `chunked_body` must end with `CR_LF`. This implies that `kValidTerm2` should not allowed in this case.
Protected by code is unused by any caller.
PiperOrigin-RevId: 789764991
diff --git a/quiche/balsa/balsa_enums.cc b/quiche/balsa/balsa_enums.cc
index 0567386..42b9e41 100644
--- a/quiche/balsa/balsa_enums.cc
+++ b/quiche/balsa/balsa_enums.cc
@@ -84,6 +84,8 @@
return "CHUNK_LENGTH_OVERFLOW";
case INVALID_CHUNK_EXTENSION:
return "INVALID_CHUNK_EXTENSION";
+ case INVALID_CHUNK_FRAMING:
+ return "INVALID_CHUNK_FRAMING";
case CALLED_BYTES_SPLICED_WHEN_UNSAFE_TO_DO_SO:
return "CALLED_BYTES_SPLICED_WHEN_UNSAFE_TO_DO_SO";
case CALLED_BYTES_SPLICED_AND_EXCEEDED_SAFE_SPLICE_AMOUNT:
diff --git a/quiche/balsa/balsa_enums.h b/quiche/balsa/balsa_enums.h
index 00e081a..f6cce3f 100644
--- a/quiche/balsa/balsa_enums.h
+++ b/quiche/balsa/balsa_enums.h
@@ -86,6 +86,7 @@
INVALID_CHUNK_LENGTH,
CHUNK_LENGTH_OVERFLOW,
INVALID_CHUNK_EXTENSION,
+ INVALID_CHUNK_FRAMING,
// Other errors.
CALLED_BYTES_SPLICED_WHEN_UNSAFE_TO_DO_SO,
diff --git a/quiche/balsa/balsa_frame.cc b/quiche/balsa/balsa_frame.cc
index 7d63d51..9c35642 100644
--- a/quiche/balsa/balsa_frame.cc
+++ b/quiche/balsa/balsa_frame.cc
@@ -1343,7 +1343,7 @@
}
const char c = *current;
- int32_t framing_found = HeaderFramingFound(c);
+ const int32_t framing_found = HeaderFramingFound(c);
if (framing_found != 0) {
// TODO(b/433557986) remove these code counts
if (framing_found == kValidTerm1 && is_request_) {
@@ -1356,9 +1356,20 @@
QUICHE_CODE_COUNT(balsa_frame_framing_found_valid_term2_response);
}
- // If we've found a "\r\n\r\n", then the message
- // is done.
+ // If we've found the end of the chunk, then we're done.
++current;
+
+ if (http_validation_policy()
+ .require_chunked_body_end_with_crlf_crlf &&
+ framing_found != kValidTerm1) {
+ // https://datatracker.ietf.org/doc/html/rfc9112#name-chunked-transfer-coding
+ // The ABNF for chunked coding states that both `last-chunk` _and_
+ // `chunked_body` must end with CR_LF, i.e. kValidTerm2 is not
+ // allowed.
+ HandleError(BalsaFrameEnums::INVALID_CHUNK_FRAMING);
+ return current - input;
+ }
+
parse_state_ = BalsaFrameEnums::MESSAGE_FULLY_READ;
visitor_->OnRawBodyInput(
absl::string_view(on_entry, current - on_entry));
diff --git a/quiche/balsa/balsa_frame_test.cc b/quiche/balsa/balsa_frame_test.cc
index 7caf5c4..b38827a 100644
--- a/quiche/balsa/balsa_frame_test.cc
+++ b/quiche/balsa/balsa_frame_test.cc
@@ -2048,6 +2048,82 @@
EXPECT_EQ(message_body, body_input);
EXPECT_EQ(message_body_data, body_data);
}
+
+// Validates that chunked requests terminated by \r\n\n are accepted.
+// Note that this does not comply with
+// https://datatracker.ietf.org/doc/html/rfc9112#name-chunked-transfer-coding
+// which states that both `last-chunk` _and_ `chunked_body` must end with CR_LF.
+TEST_F(HTTPBalsaFrameTest,
+ TransferEncodingChunkedFramesMessagesEndingWithCR_LF_LF) {
+ EXPECT_CALL(visitor_mock_, ProcessHeaders(_));
+ EXPECT_CALL(visitor_mock_, HeaderDone());
+ const std::string message1 =
+ "POST / HTTP/1.1\r\n"
+ "Host: 1.1.1.1\r\n"
+ "Transfer-Encoding: chunked\r\n"
+ "\r\n";
+ EXPECT_EQ(message1.size(),
+ balsa_frame_.ProcessInput(message1.data(), message1.size()));
+ EXPECT_FALSE(balsa_frame_.Error())
+ << BalsaFrameEnums::ErrorCodeToString(balsa_frame_.ErrorCode());
+
+ const std::string chunk_size = "2\r\n";
+ EXPECT_EQ(chunk_size.size(),
+ balsa_frame_.ProcessInput(chunk_size.data(), chunk_size.size()));
+ EXPECT_FALSE(balsa_frame_.Error())
+ << BalsaFrameEnums::ErrorCodeToString(balsa_frame_.ErrorCode());
+
+ EXPECT_CALL(visitor_mock_, OnBodyChunkInput("AA"));
+ EXPECT_CALL(visitor_mock_, MessageDone());
+ const std::string chunks =
+ "AA\r\n"
+ "0\r\n"
+ "\n";
+ ASSERT_EQ(chunks.size(),
+ balsa_frame_.ProcessInput(chunks.data(), chunks.size()));
+
+ EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode());
+ // According to the RFC, this should be false!
+ EXPECT_TRUE(balsa_frame_.MessageFullyRead());
+}
+
+TEST_F(
+ HTTPBalsaFrameTest,
+ TransferEncodingChunkedFramesMessagesEndingWithCR_LF_LFFailsWhenPolicySet) {
+ HttpValidationPolicy http_validation_policy{
+ .require_chunked_body_end_with_crlf_crlf = true};
+ balsa_frame_.set_http_validation_policy(http_validation_policy);
+ EXPECT_CALL(visitor_mock_, ProcessHeaders(_));
+ EXPECT_CALL(visitor_mock_, HeaderDone());
+ const std::string message1 =
+ "POST / HTTP/1.1\r\n"
+ "Host: 1.1.1.1\r\n"
+ "Transfer-Encoding: chunked\r\n"
+ "\r\n";
+ EXPECT_EQ(message1.size(),
+ balsa_frame_.ProcessInput(message1.data(), message1.size()));
+ EXPECT_FALSE(balsa_frame_.Error())
+ << BalsaFrameEnums::ErrorCodeToString(balsa_frame_.ErrorCode());
+
+ const std::string chunk_size = "2\r\n";
+ EXPECT_EQ(chunk_size.size(),
+ balsa_frame_.ProcessInput(chunk_size.data(), chunk_size.size()));
+ EXPECT_FALSE(balsa_frame_.Error())
+ << BalsaFrameEnums::ErrorCodeToString(balsa_frame_.ErrorCode());
+
+ EXPECT_CALL(visitor_mock_, OnBodyChunkInput("AA"));
+ EXPECT_CALL(visitor_mock_, MessageDone()).Times(0);
+ const std::string chunks =
+ "AA\r\n"
+ "0\r\n"
+ "\n";
+ ASSERT_EQ(chunks.size(),
+ balsa_frame_.ProcessInput(chunks.data(), chunks.size()));
+
+ EXPECT_EQ(BalsaFrameEnums::INVALID_CHUNK_FRAMING, balsa_frame_.ErrorCode());
+ EXPECT_FALSE(balsa_frame_.MessageFullyRead());
+}
+
TEST_F(HTTPBalsaFrameTest,
VisitorInvokedProperlyForRequestWithTransferEncodingAndTrailers) {
std::string message_headers =
diff --git a/quiche/balsa/http_validation_policy.h b/quiche/balsa/http_validation_policy.h
index 5c80f5c..80e0aec 100644
--- a/quiche/balsa/http_validation_policy.h
+++ b/quiche/balsa/http_validation_policy.h
@@ -87,6 +87,10 @@
// If true, the parser rejects messages where there is a lone LF not preceded
// by CR.
bool disallow_lone_lf_in_chunk_extension = true;
+
+ // If true, the parser rejects chunked messages that don't end with
+ // CR_LF_CR_LF.
+ bool require_chunked_body_end_with_crlf_crlf = false;
};
} // namespace quiche
