From 04bff54cee6391b12b7f44d6df4a7ea3198a08e8 Mon Sep 17 00:00:00 2001 From: Evan Zelkowitz Date: Tue, 22 Jun 2021 14:32:55 -0700 Subject: [PATCH] Fixes (#7971) Origin: https://github.com/apache/trafficserver/commit/b82a3d192f995fb9d78e1c44d51d9acca4783277 Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2021-27577 Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2021-32565 Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2021-32566 Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2021-32567 Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2021-35474 Bug-Debian: https://bugs.debian.org/990303 * String the url fragment for outgoing requests (#7966) Co-authored-by: Susan Hinrichs (cherry picked from commit 2b13eb33794574e62249997b4ba654d943a10f2d) * Ensure that the content-length value is only digits (#7964) Co-authored-by: Susan Hinrichs (cherry picked from commit 668d0f8668fec1cd350b0ceba3f7f8e4020ae3ca) * Schedule H2 reenable event only if it's necessary Co-authored-by: Katsutoshi Ikenoya * Fix dynamic-stack-buffer-overflow of cachekey plugin (#7945) * Fix dynamic-stack-buffer-overflow of cachekey plugin * Check dst_size include null termination (cherry picked from commit 5a9339d7bc65e1c2d8d2a0fc80bb051daf3cdb0b) Co-authored-by: Bryan Call Co-authored-by: Masakazu Kitajo Co-authored-by: Katsutoshi Ikenoya Co-authored-by: Masaori Koshiba Gbp-Pq: Name 0018-Fixes-7971.patch --- plugins/cachekey/cachekey.cc | 2 +- proxy/hdrs/HTTP.cc | 11 +++++++++++ proxy/http/HttpTransact.cc | 5 ++++- proxy/http2/Http2ClientSession.cc | 14 +++++++------- proxy/logging/LogUtils.cc | 2 +- 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/plugins/cachekey/cachekey.cc b/plugins/cachekey/cachekey.cc index 5f128894..44925b3d 100644 --- a/plugins/cachekey/cachekey.cc +++ b/plugins/cachekey/cachekey.cc @@ -41,7 +41,7 @@ appendEncoded(String &target, const char *s, size_t len) return; } - char tmp[len * 2]; + char tmp[len * 3 + 1]; size_t written; /* The default table does not encode the comma, so we need to use our own table here. */ diff --git a/proxy/hdrs/HTTP.cc b/proxy/hdrs/HTTP.cc index 6a2ecc41..48032dd9 100644 --- a/proxy/hdrs/HTTP.cc +++ b/proxy/hdrs/HTTP.cc @@ -1202,6 +1202,17 @@ validate_hdr_content_length(HdrHeap *heap, HTTPHdrImpl *hh) int content_length_len = 0; const char *content_length_val = content_length_field->value_get(&content_length_len); + // RFC 7230 section 3.3.2 + // Content-Length = 1*DIGIT + // + // If the content-length value contains a non-numeric value, the header is invalid + for (int i = 0; i < content_length_len; i++) { + if (!isdigit(content_length_val[i])) { + Debug("http", "Content-Length value contains non-digit, returning parse error"); + return PARSE_RESULT_ERROR; + } + } + while (content_length_field->has_dups()) { int content_length_len_2 = 0; const char *content_length_val_2 = content_length_field->m_next_dup->value_get(&content_length_len_2); diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index 66d96afe..b34d1f02 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -7610,9 +7610,12 @@ HttpTransact::build_request(State *s, HTTPHdr *base_request, HTTPHdr *outgoing_r // HttpTransactHeaders::convert_request(outgoing_version, outgoing_request); // commented out this idea + URL *url = outgoing_request->url_get(); + // Remove fragment from upstream URL + url->fragment_set(NULL, 0); + // Check whether a Host header field is missing from a 1.0 or 1.1 request. if (outgoing_version != HTTPVersion(0, 9) && !outgoing_request->presence(MIME_PRESENCE_HOST)) { - URL *url = outgoing_request->url_get(); int host_len; const char *host = url->host_get(&host_len); diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc index 6d7d3de7..ee952b8a 100644 --- a/proxy/http2/Http2ClientSession.cc +++ b/proxy/http2/Http2ClientSession.cc @@ -82,11 +82,6 @@ Http2ClientSession::destroy() void Http2ClientSession::free() { - if (this->_reenable_event) { - this->_reenable_event->cancel(); - this->_reenable_event = nullptr; - } - if (h2_pushed_urls) { this->h2_pushed_urls = ink_hash_table_destroy(this->h2_pushed_urls); } @@ -107,6 +102,11 @@ Http2ClientSession::free() REMEMBER(NO_EVENT, this->recursion) Http2SsnDebug("session free"); + if (this->_reenable_event) { + this->_reenable_event->cancel(); + this->_reenable_event = nullptr; + } + // Don't free active ProxySession ink_release_assert(is_active() == false); @@ -653,8 +653,8 @@ Http2ClientSession::remember(const SourceLocation &location, int event, int reen bool Http2ClientSession::_should_do_something_else() { - // Do something else every 128 incoming frames - return (this->_n_frame_read & 0x7F) == 0; + // Do something else every 128 incoming frames if connection state isn't closed + return (this->_n_frame_read & 0x7F) == 0 && !connection_state.is_state_closed(); } int64_t diff --git a/proxy/logging/LogUtils.cc b/proxy/logging/LogUtils.cc index 94becf25..475bee87 100644 --- a/proxy/logging/LogUtils.cc +++ b/proxy/logging/LogUtils.cc @@ -343,7 +343,7 @@ escapify_url_common(Arena *arena, char *url, size_t len_in, int *len_out, char * // size_t out_len = len_in + 2 * count; - if (dst && out_len > dst_size) { + if (dst && (out_len + 1) > dst_size) { *len_out = 0; return nullptr; } -- 2.30.2