From 724fa568435dae45ef0c3a48b2aabde052afae88 Mon Sep 17 00:00:00 2001 From: Sukchan Lee Date: Sun, 25 Sep 2022 15:06:37 +0900 Subject: [PATCH] Fixed HTTP2 crashes for random JSON data (#1769) --- lib/sbi/client.c | 40 +++++++++++++++++++++--------- lib/sbi/nghttp2-server.c | 53 +++++++++++++++++++++++++++------------- 2 files changed, 65 insertions(+), 28 deletions(-) diff --git a/lib/sbi/client.c b/lib/sbi/client.c index 8f30ac999..e4a5290ca 100644 --- a/lib/sbi/client.c +++ b/lib/sbi/client.c @@ -44,6 +44,7 @@ typedef struct connection_s { char *memory; size_t size; + bool memory_overflow; char *location; @@ -533,6 +534,8 @@ static void check_multi_info(ogs_sbi_client_t *client) res = resource->data.result; if (res == CURLE_OK) { + ogs_log_level_e level = OGS_LOG_DEBUG; + response = ogs_sbi_response_new(); ogs_assert(response); @@ -546,7 +549,17 @@ static void check_multi_info(ogs_sbi_client_t *client) response->h.uri = ogs_strdup(url); ogs_assert(response->h.uri); - ogs_debug("[%d:%s] %s", + if (content_type) + ogs_sbi_header_set(response->http.headers, + OGS_SBI_CONTENT_TYPE, content_type); + if (conn->location) + ogs_sbi_header_set(response->http.headers, + OGS_SBI_LOCATION, conn->location); + + if (conn->memory_overflow == true) + level = OGS_LOG_ERROR; + + ogs_log_message(level, 0, "[%d:%s] %s", response->status, response->h.method, response->h.uri); if (conn->memory) { @@ -557,16 +570,17 @@ static void check_multi_info(ogs_sbi_client_t *client) ogs_assert(response->http.content_length); } - ogs_debug("RECEIVED[%d]", (int)response->http.content_length); + ogs_log_message(level, 0, "RECEIVED[%d]", + (int)response->http.content_length); if (response->http.content_length && response->http.content) - ogs_debug("%s", response->http.content); + ogs_log_message(level, 0, "%s", response->http.content); + + if (conn->memory_overflow == true) { + ogs_sbi_response_free(response); + connection_remove(conn); + break; + } - if (content_type) - ogs_sbi_header_set(response->http.headers, - OGS_SBI_CONTENT_TYPE, content_type); - if (conn->location) - ogs_sbi_header_set(response->http.headers, - OGS_SBI_LOCATION, conn->location); } else ogs_warn("[%d] %s", res, conn->error); @@ -727,8 +741,12 @@ static size_t write_cb(void *contents, size_t size, size_t nmemb, void *data) realsize = size * nmemb; ptr = ogs_realloc(conn->memory, conn->size + realsize + 1); if(!ptr) { - ogs_fatal("not enough memory (realloc returned NULL)"); - ogs_assert_if_reached(); + conn->memory_overflow = true; + + ogs_error("Overflow : conn->size[%d], realsize[%d]", + (int)conn->size, (int)realsize); + ogs_log_hexdump(OGS_LOG_ERROR, contents, realsize); + return 0; } diff --git a/lib/sbi/nghttp2-server.c b/lib/sbi/nghttp2-server.c index 7272ac44e..9c6fc4248 100644 --- a/lib/sbi/nghttp2-server.c +++ b/lib/sbi/nghttp2-server.c @@ -82,6 +82,7 @@ typedef struct ogs_sbi_stream_s { int32_t stream_id; ogs_sbi_request_t *request; + bool memory_overflow; ogs_sbi_session_t *session; } ogs_sbi_stream_t; @@ -791,12 +792,23 @@ static int on_frame_recv(nghttp2_session *session, case NGHTTP2_DATA: /* HEADERS or DATA frame with +END_STREAM flag */ if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { + ogs_log_level_e level = OGS_LOG_DEBUG; - ogs_debug("[%s] %s", request->h.method, request->h.uri); + if (stream->memory_overflow == true) + level = OGS_LOG_ERROR; + + ogs_log_message(level, 0, + "[%s] %s", request->h.method, request->h.uri); if (request->http.content_length && request->http.content) { - ogs_debug("RECEIVED: %d", (int)request->http.content_length); - ogs_debug("%s", request->http.content); + ogs_log_message(level, 0, + "RECEIVED: %d", (int)request->http.content_length); + ogs_log_message(level, 0, "%s", request->http.content); + } + + if (stream->memory_overflow == true) { + ogs_error("[DROP] Overflow"); + break; } if (server->cb(request, stream) != OGS_OK) { @@ -967,23 +979,30 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, ogs_assert(len); if (request->http.content == NULL) { - request->http.content_length = len; - request->http.content = - (char*)ogs_malloc(request->http.content_length + 1); - ogs_assert(request->http.content); + ogs_assert(request->http.content_length == 0); + ogs_assert(offset == 0); + + request->http.content = (char*)ogs_malloc(len + 1); } else { - offset = request->http.content_length; - if ((request->http.content_length + len) > OGS_HUGE_LEN) { - ogs_error("Overflow : Content-Length[%d], len[%d]", - (int)request->http.content_length, (int)len); - ogs_assert_if_reached(); - } - request->http.content_length += len; - request->http.content = (char *)ogs_realloc( - request->http.content, request->http.content_length + 1); - ogs_assert(request->http.content); + ogs_assert(request->http.content_length != 0); + + request->http.content = (char*)ogs_realloc( + request->http.content, request->http.content_length + len + 1); } + if (!request->http.content) { + stream->memory_overflow = true; + + ogs_error("Overflow : Content-Length[%d], len[%d]", + (int)request->http.content_length, (int)len); + ogs_log_hexdump(OGS_LOG_ERROR, data, len); + + return 0; + } + + offset = request->http.content_length; + request->http.content_length += len; + memcpy(request->http.content + offset, data, len); request->http.content[request->http.content_length] = '\0';