Skip to content

Commit eaef5e8

Browse files
masaori335maskit
andauthored
Fail fast on HTTP/2 header validation (#9009)
Co-authored-by: Masakazu Kitajo <maskit@apache.org>
1 parent 697da39 commit eaef5e8

File tree

3 files changed

+19
-12
lines changed

3 files changed

+19
-12
lines changed

proxy/hdrs/MIME.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ struct MIMEField {
170170
int value_get_comma_list(StrList *list) const;
171171

172172
void name_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *name, int length);
173-
bool name_is_valid() const;
173+
bool name_is_valid(uint32_t invalid_char_bits = is_control_BIT) const;
174174

175175
void value_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int length);
176176
void value_set_int(HdrHeap *heap, MIMEHdrImpl *mh, int32_t value);
@@ -182,7 +182,7 @@ struct MIMEField {
182182
// Other separators (e.g. ';' in Set-cookie/Cookie) are also possible
183183
void value_append(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int length, bool prepend_comma = false,
184184
const char separator = ',');
185-
bool value_is_valid() const;
185+
bool value_is_valid(uint32_t invalid_char_bits = is_control_BIT) const;
186186
int has_dups() const;
187187
};
188188

@@ -972,13 +972,13 @@ MIMEField::name_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *name, int length
972972
-------------------------------------------------------------------------*/
973973

974974
inline bool
975-
MIMEField::name_is_valid() const
975+
MIMEField::name_is_valid(uint32_t invalid_char_bits) const
976976
{
977977
const char *name;
978978
int length;
979979

980980
for (name = name_get(&length); length > 0; length--) {
981-
if (ParseRules::is_control(name[length - 1])) {
981+
if (ParseRules::is_type(name[length - 1], invalid_char_bits)) {
982982
return false;
983983
}
984984
}
@@ -1081,13 +1081,13 @@ MIMEField::value_append(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int l
10811081
-------------------------------------------------------------------------*/
10821082

10831083
inline bool
1084-
MIMEField::value_is_valid() const
1084+
MIMEField::value_is_valid(uint32_t invalid_char_bits) const
10851085
{
10861086
const char *value;
10871087
int length;
10881088

10891089
for (value = value_get(&length); length > 0; length--) {
1090-
if (ParseRules::is_control(value[length - 1])) {
1090+
if (ParseRules::is_type(value[length - 1], invalid_char_bits)) {
10911091
return false;
10921092
}
10931093
}

proxy/http2/HTTP2.cc

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,8 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
430430

431431
if (http_hdr_type_get(headers->m_http) == HTTP_TYPE_REQUEST) {
432432
// :scheme
433-
if (MIMEField *field = headers->field_find(HTTP2_VALUE_SCHEME, HTTP2_LEN_SCHEME); field != nullptr && field->value_is_valid()) {
433+
if (MIMEField *field = headers->field_find(HTTP2_VALUE_SCHEME, HTTP2_LEN_SCHEME);
434+
field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
434435
int scheme_len;
435436
const char *scheme = field->value_get(&scheme_len);
436437

@@ -444,7 +445,7 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
444445

445446
// :authority
446447
if (MIMEField *field = headers->field_find(HTTP2_VALUE_AUTHORITY, HTTP2_LEN_AUTHORITY);
447-
field != nullptr && field->value_is_valid()) {
448+
field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
448449
int authority_len;
449450
const char *authority = field->value_get(&authority_len);
450451

@@ -456,7 +457,8 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
456457
}
457458

458459
// :path
459-
if (MIMEField *field = headers->field_find(HTTP2_VALUE_PATH, HTTP2_LEN_PATH); field != nullptr && field->value_is_valid()) {
460+
if (MIMEField *field = headers->field_find(HTTP2_VALUE_PATH, HTTP2_LEN_PATH);
461+
field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
460462
int path_len;
461463
const char *path = field->value_get(&path_len);
462464

@@ -474,7 +476,8 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
474476
}
475477

476478
// :method
477-
if (MIMEField *field = headers->field_find(HTTP2_VALUE_METHOD, HTTP2_LEN_METHOD); field != nullptr && field->value_is_valid()) {
479+
if (MIMEField *field = headers->field_find(HTTP2_VALUE_METHOD, HTTP2_LEN_METHOD);
480+
field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
478481
int method_len;
479482
const char *method = field->value_get(&method_len);
480483

@@ -503,7 +506,7 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
503506

504507
// Check validity of all names and values
505508
for (auto &mf : *headers) {
506-
if (!mf.name_is_valid() || !mf.value_is_valid()) {
509+
if (!mf.name_is_valid(is_control_BIT | is_ws_BIT) || !mf.value_is_valid()) {
507510
return PARSE_RESULT_ERROR;
508511
}
509512
}

proxy/http2/Http2Stream.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,11 @@ Http2Stream::send_request(Http2ConnectionState &cstate)
236236
this->_http_sm_id = this->_sm->sm_id;
237237

238238
// Convert header to HTTP/1.1 format
239-
http2_convert_header_from_2_to_1_1(&_req_header);
239+
if (http2_convert_header_from_2_to_1_1(&_req_header) == PARSE_RESULT_ERROR) {
240+
// There's no way to cause Bad Request directly at this time.
241+
// Set an invalid method so it causes an error later.
242+
_req_header.method_set("\xffVOID", 1);
243+
}
240244

241245
// Write header to a buffer. Borrowing logic from HttpSM::write_header_into_buffer.
242246
// Seems like a function like this ought to be in HTTPHdr directly

0 commit comments

Comments
 (0)