Repository: trafficserver Updated Branches: refs/heads/master 7dbb358c9 -> fe2de1a3d
TS-4019: Validate header names and values before passing to FetchSM This closes #334 Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/fe2de1a3 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/fe2de1a3 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/fe2de1a3 Branch: refs/heads/master Commit: fe2de1a3d552c09409bfc328a2dfe6a4f1691930 Parents: 7dbb358 Author: Masakazu Kitajo <[email protected]> Authored: Sat Nov 14 13:02:33 2015 -0800 Committer: Bryan Call <[email protected]> Committed: Sat Nov 14 13:02:33 2015 -0800 ---------------------------------------------------------------------- proxy/hdrs/MIME.h | 37 ++++++++++++++++++++++++++++++++ proxy/http2/HTTP2.cc | 16 ++++++++++---- proxy/http2/Http2ConnectionState.cc | 4 +++- proxy/http2/Http2Stream.cc | 7 ++++-- proxy/http2/Http2Stream.h | 2 +- 5 files changed, 58 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fe2de1a3/proxy/hdrs/MIME.h ---------------------------------------------------------------------- diff --git a/proxy/hdrs/MIME.h b/proxy/hdrs/MIME.h index 903bfc3..f882df1 100644 --- a/proxy/hdrs/MIME.h +++ b/proxy/hdrs/MIME.h @@ -29,6 +29,7 @@ #include "ts/ink_assert.h" #include "ts/ink_apidefs.h" #include "ts/ink_string++.h" +#include "ts/ParseRules.h" #include "HdrHeap.h" #include "HdrToken.h" @@ -164,6 +165,7 @@ struct MIMEField { int value_get_comma_list(StrList *list) const; void name_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *name, int length); + bool name_is_valid() const; void value_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int length); void value_set_int(HdrHeap *heap, MIMEHdrImpl *mh, int32_t value); @@ -175,6 +177,7 @@ struct MIMEField { // Other separators (e.g. ';' in Set-cookie/Cookie) are also possible void value_append(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int length, bool prepend_comma = false, const char separator = ','); + bool value_is_valid() const; int has_dups() const; }; @@ -765,6 +768,23 @@ MIMEField::name_set(HdrHeap *heap, MIMEHdrImpl *mh, const char *name, int length /*------------------------------------------------------------------------- -------------------------------------------------------------------------*/ +inline bool +MIMEField::name_is_valid() const +{ + const char *name; + int length; + + for (name = name_get(&length); length > 0; length--) { + if (ParseRules::is_control(name[length - 1])) { + return false; + } + } + return true; +} + +/*------------------------------------------------------------------------- + -------------------------------------------------------------------------*/ + inline const char * MIMEField::value_get(int *length) const { @@ -852,6 +872,23 @@ MIMEField::value_append(HdrHeap *heap, MIMEHdrImpl *mh, const char *value, int l mime_field_value_append(heap, mh, this, value, length, prepend_comma, separator); } +/*------------------------------------------------------------------------- + -------------------------------------------------------------------------*/ + +inline bool +MIMEField::value_is_valid() const +{ + const char *value; + int length; + + for (value = value_get(&length); length > 0; length--) { + if (ParseRules::is_control(value[length - 1])) { + return false; + } + } + return true; +} + inline int MIMEField::has_dups() const { http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fe2de1a3/proxy/http2/HTTP2.cc ---------------------------------------------------------------------- diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc index 16a72fa..7789e90 100644 --- a/proxy/http2/HTTP2.cc +++ b/proxy/http2/HTTP2.cc @@ -405,19 +405,19 @@ convert_from_2_to_1_1_header(HTTPHdr *headers) int scheme_len, authority_len, path_len, method_len; // Get values of :scheme, :authority and :path to assemble requested URL - if ((field = headers->field_find(HPACK_VALUE_SCHEME, HPACK_LEN_SCHEME)) != NULL) { + if ((field = headers->field_find(HPACK_VALUE_SCHEME, HPACK_LEN_SCHEME)) != NULL && field->value_is_valid()) { scheme = field->value_get(&scheme_len); } else { return PARSE_ERROR; } - if ((field = headers->field_find(HPACK_VALUE_AUTHORITY, HPACK_LEN_AUTHORITY)) != NULL) { + if ((field = headers->field_find(HPACK_VALUE_AUTHORITY, HPACK_LEN_AUTHORITY)) != NULL && field->value_is_valid()) { authority = field->value_get(&authority_len); } else { return PARSE_ERROR; } - if ((field = headers->field_find(HPACK_VALUE_PATH, HPACK_LEN_PATH)) != NULL) { + if ((field = headers->field_find(HPACK_VALUE_PATH, HPACK_LEN_PATH)) != NULL && field->value_is_valid()) { path = field->value_get(&path_len); } else { return PARSE_ERROR; @@ -437,7 +437,7 @@ convert_from_2_to_1_1_header(HTTPHdr *headers) arena.str_free(url); // Get value of :method - if ((field = headers->field_find(HPACK_VALUE_METHOD, HPACK_LEN_METHOD)) != NULL) { + if ((field = headers->field_find(HPACK_VALUE_METHOD, HPACK_LEN_METHOD)) != NULL && field->value_is_valid()) { method = field->value_get(&method_len); int method_wks_idx = hdrtoken_tokenize(method, method_len); @@ -476,6 +476,14 @@ convert_from_2_to_1_1_header(HTTPHdr *headers) headers->field_delete(HPACK_VALUE_STATUS, HPACK_LEN_STATUS); } + // Check validity of all names and values + MIMEFieldIter iter; + for (const MIMEField *field = headers->iter_get_first(&iter); field != NULL; field = headers->iter_get_next(&iter)) { + if (!field->name_is_valid() || !field->value_is_valid()) { + return PARSE_ERROR; + } + } + return PARSE_DONE; } http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fe2de1a3/proxy/http2/Http2ConnectionState.cc ---------------------------------------------------------------------- diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc index 39d21f6..0f950e7 100644 --- a/proxy/http2/Http2ConnectionState.cc +++ b/proxy/http2/Http2ConnectionState.cc @@ -259,7 +259,9 @@ rcv_headers_frame(Http2ClientSession &cs, Http2ConnectionState &cstate, const Ht return Http2Error(HTTP2_ERROR_CLASS_STREAM, HTTP2_ERROR_PROTOCOL_ERROR); } - stream->init_fetcher(cstate); + if (!stream->init_fetcher(cstate)) { + return Http2Error(HTTP2_ERROR_CLASS_STREAM, HTTP2_ERROR_PROTOCOL_ERROR); + } } else { // NOTE: Expect CONTINUATION Frame. Do NOT change state of stream or decode // Header Blocks. http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fe2de1a3/proxy/http2/Http2Stream.cc ---------------------------------------------------------------------- diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc index 2ea7020..30c175f 100644 --- a/proxy/http2/Http2Stream.cc +++ b/proxy/http2/Http2Stream.cc @@ -28,13 +28,15 @@ // Currently use only HTTP/1.1 for requesting to origin server const static char *HTTP2_FETCHING_HTTP_VERSION = "HTTP/1.1"; -void +bool Http2Stream::init_fetcher(Http2ConnectionState &cstate) { extern ClassAllocator<FetchSM> FetchSMAllocator; // Convert header to HTTP/1.1 format - convert_from_2_to_1_1_header(&_req_header); + if (convert_from_2_to_1_1_header(&_req_header) == PARSE_ERROR) { + return false; + } // Get null-terminated URL and method Arena arena; @@ -61,6 +63,7 @@ Http2Stream::init_fetcher(Http2ConnectionState &cstate) _fetch_sm->ext_set_user_data(this); _fetch_sm->ext_launch(); + return true; } void http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fe2de1a3/proxy/http2/Http2Stream.h ---------------------------------------------------------------------- diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h index f47f096..c96c434 100644 --- a/proxy/http2/Http2Stream.h +++ b/proxy/http2/Http2Stream.h @@ -61,7 +61,7 @@ public: } // Operate FetchSM - void init_fetcher(Http2ConnectionState &cstate); + bool init_fetcher(Http2ConnectionState &cstate); void set_body_to_fetcher(const void *data, size_t len); FetchSM * get_fetcher()
