This is an automated email from the ASF dual-hosted git repository. masaori pushed a commit to branch asf-master-0809-2 in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 2424565f96fe861e52f31e241ced87c73c5fe165 Author: Masaori Koshiba <masa...@apache.org> AuthorDate: Tue Mar 29 14:37:16 2022 +0900 Restrict unknown scheme of HTTP/2 request 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 f1360ddc6..df31bc932 100644 --- a/proxy/http2/HTTP2.cc +++ b/proxy/http2/HTTP2.cc @@ -433,8 +433,17 @@ http2_convert_header_from_2_to_1_1(HTTPHdr *headers) if (MIMEField *field = headers->field_find(HTTP2_VALUE_SCHEME, HTTP2_LEN_SCHEME); field != nullptr && field->value_is_valid()) { 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);