This is an automated email from the ASF dual-hosted git repository.
eze pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/8.1.x by this push:
new 8e9a818d24 Stricten field name check (#11597)
8e9a818d24 is described below
commit 8e9a818d24f049b0b1b712aa8e5f5cf4a2440871
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Tue Jul 23 12:07:33 2024 -0600
Stricten field name check (#11597)
* Stricten field name check
* clang-format
* Remove invalid tests
---
include/tscore/ParseRules.h | 7 ++++---
proxy/hdrs/HdrTest.cc | 2 --
proxy/hdrs/MIME.cc | 3 ++-
proxy/hdrs/unit_tests/test_Hdrs.cc | 19 +++++++++++++++++++
4 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/tscore/ParseRules.h b/include/tscore/ParseRules.h
index eff74a16e5..77d734d617 100644
--- a/include/tscore/ParseRules.h
+++ b/include/tscore/ParseRules.h
@@ -127,8 +127,8 @@ public:
static CTypeResult is_alnum(char c); // 0-9,A-Z,a-z
static CTypeResult is_space(char c); // ' ' HT,VT,NP,CR,LF
static CTypeResult is_control(char c); // 0x00-0x08, 0x0a-0x1f, 0x7f
- static CTypeResult is_mime_sep(char c); // ()<>,;\"/[]?{} \t
- static CTypeResult is_http_field_name(char c); // not : or mime_sep except
for @
+ static CTypeResult is_mime_sep(char c); // @()<>,;\"/[]?{} \t
+ static CTypeResult is_http_field_name(char c); // not :, =, 0x80+, control,
or mime_sep except for @
static CTypeResult is_http_field_value(char c); // not CR, LF, comma, or "
//////////////////
@@ -706,8 +706,9 @@ ParseRules::is_http_field_name(char c)
#ifndef COMPILE_PARSE_RULES
return (parseRulesCType[(unsigned char)c] & is_http_field_name_BIT);
#else
- if ((c == ':') || (is_mime_sep(c) && (c != '@')))
+ if (!is_char(c) || is_control(c) || (is_mime_sep(c) && c != '@') || c == '='
|| c == ':') {
return false;
+ }
return true;
#endif
}
diff --git a/proxy/hdrs/HdrTest.cc b/proxy/hdrs/HdrTest.cc
index 8046e69aa4..dd0962f598 100644
--- a/proxy/hdrs/HdrTest.cc
+++ b/proxy/hdrs/HdrTest.cc
@@ -543,13 +543,11 @@ HdrTest::test_mime()
"Cache-Control: private\r\n"
"accept: foo\r\n"
"accept: bar\n"
- ": (null) field name\r\n"
"aCCept: \n"
"ACCEPT\r\n"
"foo: bar\r\n"
"foo: argh\r\n"
"foo: three, four\r\n"
- "word word: word \r\n"
"accept: \"fazzle, dazzle\"\r\n"
"accept: 1, 2, 3, 4, 5, 6, 7, 8\r\n"
"continuation: part1\r\n"
diff --git a/proxy/hdrs/MIME.cc b/proxy/hdrs/MIME.cc
index f010b6ad50..0136b923be 100644
--- a/proxy/hdrs/MIME.cc
+++ b/proxy/hdrs/MIME.cc
@@ -2695,7 +2695,7 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap,
MIMEHdrImpl *mh, const char
if (field_name_wks_idx < 0) {
for (int i = 0; i < field_name_length; ++i) {
- if (ParseRules::is_control(field_name_first[i])) {
+ if (!ParseRules::is_http_field_name(field_name_first[i])) {
return PARSE_RESULT_ERROR;
}
}
@@ -2703,6 +2703,7 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap,
MIMEHdrImpl *mh, const char
// RFC 9110 Section 5.5. Field Values
for (int i = 0; i < field_value_length; ++i) {
+ // FIXME: ParseRules::is_http_field_value() should be used but the
implementation looks wrong
if (ParseRules::is_control(field_value_first[i])) {
return PARSE_RESULT_ERROR;
}
diff --git a/proxy/hdrs/unit_tests/test_Hdrs.cc
b/proxy/hdrs/unit_tests/test_Hdrs.cc
index 7990f3e360..b98298f42a 100644
--- a/proxy/hdrs/unit_tests/test_Hdrs.cc
+++ b/proxy/hdrs/unit_tests/test_Hdrs.cc
@@ -41,6 +41,25 @@ TEST_CASE("HdrTest", "[proxy][hdrtest]")
// Field Name
{"Content-Length: 10\r\n", PARSE_RESULT_CONT},
{"Content-Length\x0b: 10\r\n", PARSE_RESULT_ERROR},
+ {"Content-Length\xff: 10\r\n", PARSE_RESULT_ERROR},
+ // Delimiters in field name
+ {"delimiter_\": 10\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_(: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_): 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_,: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_/: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_:: 0\r\n", PARSE_RESULT_CONT}, // Parsed as field name
"delimiter_" and field value ": 0", which is valid
+ {"delimiter_;: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_<: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_=: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_>: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_?: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_@: 0\r\n", PARSE_RESULT_CONT}, // Not allowed by the spec,
but we use it as internal header indicator
+ {"delimiter_[: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_\\: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_]: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_{: 0\r\n", PARSE_RESULT_ERROR},
+ {"delimiter_}: 0\r\n", PARSE_RESULT_ERROR},
////
// Field Value
// SP
