This is an automated email from the ASF dual-hosted git repository.

cmcfarlen pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 847b7a781eb27b04728c31ddfdc76c8f0ef79fc9
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Tue Jul 23 11:19:55 2024 -0600

    Stricten field name check (#11593)
    
    * Stricten field name check
    
    * Add a test
    
    (cherry picked from commit b104992e2315969688a697cbf7d5007a7dca396f)
---
 include/tscore/ParseRules.h            |  6 +++---
 src/proxy/hdrs/MIME.cc                 |  3 ++-
 src/proxy/hdrs/unit_tests/test_Hdrs.cc | 21 +++++++++++++++++++--
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/include/tscore/ParseRules.h b/include/tscore/ParseRules.h
index ab8d0c8356..9299b411c9 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 "
 
   //////////////////
@@ -712,7 +712,7 @@ ParseRules::is_http_field_name(char c)
 #ifndef COMPILE_PARSE_RULES
   return (parseRulesCType[static_cast<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;
diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc
index 5194148755..75f777fd27 100644
--- a/src/proxy/hdrs/MIME.cc
+++ b/src/proxy/hdrs/MIME.cc
@@ -2631,7 +2631,7 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, 
MIMEHdrImpl *mh, const char
 
     if (field_name_wks_idx < 0) {
       for (auto i : field_name) {
-        if (ParseRules::is_control(i)) {
+        if (!ParseRules::is_http_field_name(i)) {
           return PARSE_RESULT_ERROR;
         }
       }
@@ -2639,6 +2639,7 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, 
MIMEHdrImpl *mh, const char
 
     // RFC 9110 Section 5.5. Field Values
     for (char i : field_value) {
+      // FIXME: ParseRules::is_http_field_value() should be used but the 
implementation looks wrong
       if (ParseRules::is_control(i)) {
         return PARSE_RESULT_ERROR;
       }
diff --git a/src/proxy/hdrs/unit_tests/test_Hdrs.cc 
b/src/proxy/hdrs/unit_tests/test_Hdrs.cc
index 5ab545989f..0bdc7faca5 100644
--- a/src/proxy/hdrs/unit_tests/test_Hdrs.cc
+++ b/src/proxy/hdrs/unit_tests/test_Hdrs.cc
@@ -554,6 +554,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
@@ -891,13 +910,11 @@ TEST_CASE("HdrTest", "[proxy][hdrtest]")
       "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"

Reply via email to