From 081a7974e63fa61b717c2ed96c4bfd21d3e4c866 Mon Sep 17 00:00:00 2001 From: Philipp Schafft Date: Wed, 17 Oct 2018 10:23:33 +0000 Subject: [PATCH 1/3] Fix: Fixed buffer overflow in URL auth code. Closes: #2342 --- src/auth_url.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/auth_url.c b/src/auth_url.c index 6b3c3789..ec26794a 100644 --- a/src/auth_url.c +++ b/src/auth_url.c @@ -513,13 +513,25 @@ static auth_result url_add_client(auth_client *auth_user) header_val = httpp_getvar (client->parser, cur_header); if (header_val) { + size_t left = sizeof(post) - post_offset; + int ret; + header_valesc = util_url_escape (header_val); - post_offset += snprintf(post + post_offset, + ret = snprintf(post + post_offset, sizeof(post) - post_offset, "&%s%s=%s", url->prefix_headers ? url->prefix_headers : "", cur_header, header_valesc); free(header_valesc); + + if (ret <= 0 || (size_t)ret >= left) { + ICECAST_LOG_ERROR("Authentication failed for client %p as header \"%H\" is too long.", client, cur_header); + free(pass_headers); + auth_user_url_clear(auth_user); + return AUTH_FAILED; + } else { + post_offset += ret; + } } cur_header = next_header; From 548e7963a76ba8e147d3f4bbe2c90d114b13b676 Mon Sep 17 00:00:00 2001 From: Philipp Schafft Date: Wed, 17 Oct 2018 11:25:06 +0000 Subject: [PATCH 2/3] Fix: Fixed bufferoverflow within url_add_client() This can be trigged by: * overly long username, * overly long password, * overly long user agent string, * overly long path. --- src/auth_url.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/auth_url.c b/src/auth_url.c index ec26794a..7625dc81 100644 --- a/src/auth_url.c +++ b/src/auth_url.c @@ -343,6 +343,7 @@ static auth_result url_remove_client(auth_client *auth_user) const char *agent; char *user_agent, *ipaddr; + int ret; if (url->removeurl == NULL) return AUTH_OK; @@ -378,7 +379,7 @@ static auth_result url_remove_client(auth_client *auth_user) mount = util_url_escape(mountreq); ipaddr = util_url_escape(client->con->ip); - snprintf(post, sizeof (post), + ret = snprintf(post, sizeof(post), "action=%s&server=%s&port=%d&client=%lu&mount=%s" "&user=%s&pass=%s&duration=%lu&ip=%s&agent=%s", url->removeaction, /* already escaped */ @@ -392,6 +393,12 @@ static auth_result url_remove_client(auth_client *auth_user) free(ipaddr); free(user_agent); + if (ret <= 0 || ret >= (ssize_t)sizeof(post)) { + ICECAST_LOG_ERROR("Authentication failed for client %p as header POST data is too long.", client); + auth_user_url_clear(auth_user); + return AUTH_FAILED; + } + if (strchr (url->removeurl, '@') == NULL) { if (url->userpwd) { curl_easy_setopt(url->handle, CURLOPT_USERPWD, url->userpwd); @@ -499,6 +506,13 @@ static auth_result url_add_client(auth_client *auth_user) free(password); free(ipaddr); + + if (post_offset <= 0 || post_offset >= (ssize_t)sizeof(post)) { + ICECAST_LOG_ERROR("Authentication failed for client %p as header POST data is too long.", client); + auth_user_url_clear(auth_user); + return AUTH_FAILED; + } + pass_headers = NULL; if (url->pass_headers) pass_headers = strdup(url->pass_headers); From 162e3dd650e211381e110c6f64541de08a05312c Mon Sep 17 00:00:00 2001 From: Philipp Schafft Date: Wed, 17 Oct 2018 13:07:48 +0000 Subject: [PATCH 3/3] Fix: Corrected possible bufferoverflows in format_prepare_headers() --- src/format.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/format.c b/src/format.c index 34cf8449..61ecf7c4 100644 --- a/src/format.c +++ b/src/format.c @@ -295,7 +295,7 @@ static inline ssize_t __print_var(char *str, size_t remaining, const char *forma for (i = 0; i < var->values; i++) { ret = snprintf(str + done, remaining - done, format, first, var->value[i]); - if (ret == -1) + if (ret <= 0 || (size_t)ret >= (remaining - done)) return -1; done += ret; @@ -331,7 +331,7 @@ static int format_prepare_headers (source_t *source, client_t *client) client->respcode = 200; bytes = util_http_build_header(ptr, remaining, 0, 0, 200, NULL, source->format->contenttype, NULL, NULL, source, client); - if (bytes < 0) { + if (bytes <= 0) { ICECAST_LOG_ERROR("Dropping client as we can not build response headers."); client->respcode = 500; return -1; @@ -342,7 +342,7 @@ static int format_prepare_headers (source_t *source, client_t *client) client->refbuf->data = ptr = new_ptr; client->refbuf->len = remaining = bytes + 1024; bytes = util_http_build_header(ptr, remaining, 0, 0, 200, NULL, source->format->contenttype, NULL, NULL, source, client); - if (bytes == -1 ) { + if (bytes <= 0 || (size_t)bytes >= remaining) { ICECAST_LOG_ERROR("Dropping client as we can not build response headers."); client->respcode = 500; return -1; @@ -354,6 +354,11 @@ static int format_prepare_headers (source_t *source, client_t *client) } } + if (bytes <= 0 || (size_t)bytes >= remaining) { + ICECAST_LOG_ERROR("Can not allocate headers for client %p", client); + client->respcode = 500; + return -1; + } remaining -= bytes; ptr += bytes; @@ -421,6 +426,13 @@ static int format_prepare_headers (source_t *source, client_t *client) } } + if (bytes < 0 || (size_t)bytes >= remaining) { + avl_tree_unlock(source->parser->vars); + ICECAST_LOG_ERROR("Can not allocate headers for client %p", client); + client->respcode = 500; + return -1; + } + remaining -= bytes; ptr += bytes; if (next) @@ -429,6 +441,11 @@ static int format_prepare_headers (source_t *source, client_t *client) avl_tree_unlock(source->parser->vars); bytes = snprintf(ptr, remaining, "\r\n"); + if (bytes <= 0 || (size_t)bytes >= remaining) { + ICECAST_LOG_ERROR("Can not allocate headers for client %p", client); + client->respcode = 500; + return -1; + } remaining -= bytes; ptr += bytes;