[trafficserver] branch master updated: Restrict unknown scheme of HTTP/2 request (#9010)

[bc...@apache.org]

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

bcall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new c56f87224 Restrict unknown scheme of HTTP/2 request (#9010)
c56f87224 is described below

commit c56f8722470a2e068557006ed6750c545602a62c
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Aug 9 09:29:20 2022 +0900

    Restrict unknown scheme of HTTP/2 request (#9010)
    
    Strictly following RFC 3986 Section 3.1
    
    ```
    scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
    ```
---
 proxy/hdrs/URL.cc                 | 42 +++++++++++++++++++++++++++++----------
 proxy/hdrs/URL.h                  |  2 ++
 proxy/hdrs/unit_tests/test_URL.cc | 21 ++++++++++++++++++++
 proxy/http2/HTTP2.cc              | 11 +++++++++-
 4 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/proxy/hdrs/URL.cc b/proxy/hdrs/URL.cc
index 5614a9b9c..2896cdfb9 100644
--- a/proxy/hdrs/URL.cc
+++ b/proxy/hdrs/URL.cc
@@ -114,6 +114,36 @@ validate_host_name(std::string_view addr)
   return std::all_of(addr.begin(), addr.end(), &is_host_char);
 }
 
+/**
+   Checks if the (un-well-known) scheme is valid
+
+   RFC 3986 Section 3.1
+   These are the valid characters in a scheme:
+     scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
+   return an error if there is another character in the scheme
+*/
+bool
+validate_scheme(std::string_view scheme)
+{
+  if (scheme.empty()) {
+    return false;
+  }
+
+  if (!ParseRules::is_alpha(scheme[0])) {
+    return false;
+  }
+
+  for (size_t i = 0; i < scheme.size(); ++i) {
+    const char &c = scheme[i];
+
+    if (!(ParseRules::is_alnum(c) != 0 || c == '+' || c == '-' || c == '.')) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 /*-------------------------------------------------------------------------
   -------------------------------------------------------------------------*/
 
@@ -1133,19 +1163,9 @@ url_parse_scheme(HdrHeap *heap, URLImpl *url, const char **start, const char *en
 
         if (!(scheme_wks_idx > 0 && hdrtoken_wks_to_token_type(scheme_wks) == HDRTOKEN_TYPE_SCHEME)) {
           // Unknown scheme, validate the scheme
-
-          // RFC 3986 Section 3.1
-          // These are the valid characters in a scheme:
-          //   scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
-          // return an error if there is another character in the scheme
-          if (!ParseRules::is_alpha(*scheme_start)) {
+          if (!validate_scheme({scheme_start, static_cast<size_t>(scheme_end - scheme_start)})) {
             return PARSE_RESULT_ERROR;
           }
-          for (cur = scheme_start + 1; cur < scheme_end; ++cur) {
-            if (!(ParseRules::is_alnum(*cur) != 0 || *cur == '+' || *cur == '-' || *cur == '.')) {
-              return PARSE_RESULT_ERROR;
-            }
-          }
         }
         url->set_scheme(heap, scheme_start, scheme_wks_idx, scheme_end - scheme_start, copy_strings_p);
       }
diff --git a/proxy/hdrs/URL.h b/proxy/hdrs/URL.h
index ac9d4a0d9..c9a1c75f5 100644
--- a/proxy/hdrs/URL.h
+++ b/proxy/hdrs/URL.h
@@ -185,6 +185,8 @@ extern int URL_LEN_MMST;
 
 /* Public */
 bool validate_host_name(std::string_view addr);
+bool validate_scheme(std::string_view scheme);
+
 void url_init();
 
 URLImpl *url_create(HdrHeap *heap);
diff --git a/proxy/hdrs/unit_tests/test_URL.cc b/proxy/hdrs/unit_tests/test_URL.cc
index a39f7c01b..115aeee5d 100644
--- a/proxy/hdrs/unit_tests/test_URL.cc
+++ b/proxy/hdrs/unit_tests/test_URL.cc
@@ -53,6 +53,27 @@ TEST_CASE("ValidateURL", "[proxy][validurl]")
   }
 }
 
+TEST_CASE("Validate Scheme", "[proxy][validscheme]")
+{
+  static const struct {
+    std::string_view text;
+    bool valid;
+  } scheme_test_cases[] = {{"http", true},      {"https", true},      {"example", true},    {"example.", true},
+                           {"example++", true}, {"example--.", true}, {"++example", false}, {"--example", false},
+                           {".example", false}, {"example://", false}};
+
+  for (auto i : scheme_test_cases) {
+    // it's pretty hard to debug with
+    //     CHECK(validate_scheme(i.text) == i.valid);
+
+    std::string_view text = i.text;
+    if (validate_scheme(text) != i.valid) {
+      std::printf("Validation of scheme: \"%s\", expected %s, but not\n", text.data(), (i.valid ? "true" : "false"));
+      CHECK(false);
+    }
+  }
+}
+
 namespace UrlImpl
 {
 bool url_is_strictly_compliant(const char *start, const char *end);
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index 73cc4022e..5b263bbe7 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -434,8 +434,17 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers)
         field != nullptr && field->value_is_valid(is_control_BIT | is_ws_BIT)) {
       int scheme_len;
       const char *scheme = field->value_get(&scheme_len);
+      const char *scheme_wks;
+
+      int scheme_wks_idx = hdrtoken_tokenize(scheme, scheme_len, &scheme_wks);
+
+      if (!(scheme_wks_idx > 0 && hdrtoken_wks_to_token_type(scheme_wks) == HDRTOKEN_TYPE_SCHEME)) {
+        // unkown scheme, validate the scheme
+        if (!validate_scheme({scheme, static_cast<size_t>(scheme_len)})) {
+          return PARSE_RESULT_ERROR;
+        }
+      }
 
-      int scheme_wks_idx = hdrtoken_tokenize(scheme, scheme_len);
       headers->m_http->u.req.m_url_impl->set_scheme(headers->m_heap, scheme, scheme_wks_idx, scheme_len, true);
 
       headers->field_delete(field);