You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2022/08/08 23:18:16 UTC
[trafficserver] 01/01: Restrict unknown scheme of HTTP/2 request
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 <ma...@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);