From cd731fcaf42e5f7078c9be643bfa0cee2ad53e8f Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 7 Nov 2023 10:33:21 +0100 Subject: [PATCH] detect: fixes use-after-free with http.request_header Ticket: #6441 This keyword and the response one use a multiple inspection buffer. But the different instances point to the same memory address that comes from HttpHeaderGetBufferSpace and is not owned by the transaction, and is rebuilt, which is a functional bug in itself. As it gets crafted, it can get reallocated if one header is over 1024 bytes, while the previous freed pointer will still get used for the previous headers. (cherry picked from commit bc422c17d6961f03f673f2999a949913e89fc2d0) --- src/detect-http-header.c | 114 ++++++++++++++++++++++++++++++--------- 1 file changed, 88 insertions(+), 26 deletions(-) diff --git a/src/detect-http-header.c b/src/detect-http-header.c index e5101f9276b..2803d05a5cf 100644 --- a/src/detect-http-header.c +++ b/src/detect-http-header.c @@ -48,6 +48,7 @@ #include "util-print.h" #include "util-memcmp.h" #include "util-profiling.h" +#include "util-validate.h" #include "app-layer.h" #include "app-layer-parser.h" @@ -467,6 +468,8 @@ void DetectHttpHeaderRegister(void) static int g_http_request_header_buffer_id = 0; static int g_http_response_header_buffer_id = 0; +static int g_request_header_thread_id = 0; +static int g_response_header_thread_id = 0; static InspectionBuffer *GetHttp2HeaderData(DetectEngineThreadCtx *det_ctx, const uint8_t flags, const DetectEngineTransforms *transforms, Flow *_f, const struct MpmListIdDataArgs *cbdata, @@ -580,6 +583,36 @@ static int PrefilterMpmHttp2HeaderRegister(DetectEngineCtx *de_ctx, SigGroupHead mpm_reg->app_v2.tx_min_progress, pectx, PrefilterMpmHttpHeaderFree, mpm_reg->name); } +typedef struct HttpMultiBufItem { + uint8_t *buffer; + size_t len; +} HttpMultiBufItem; + +typedef struct HttpMultiBufHeaderThreadData { + // array of items, being defined as a buffer with its length just above + HttpMultiBufItem *items; + // capacity of items (size of allocation) + size_t cap; + // length of items (number in use) + size_t len; +} HttpMultiBufHeaderThreadData; + +static void *HttpMultiBufHeaderThreadDataInit(void *data) +{ + HttpMultiBufHeaderThreadData *td = SCCalloc(1, sizeof(*td)); + return td; +} + +static void HttpMultiBufHeaderThreadDataFree(void *data) +{ + HttpMultiBufHeaderThreadData *td = data; + for (size_t i = 0; i < td->cap; i++) { + SCFree(td->items[i].buffer); + } + SCFree(td->items); + SCFree(td); +} + static InspectionBuffer *GetHttp1HeaderData(DetectEngineThreadCtx *det_ctx, const uint8_t flags, const DetectEngineTransforms *transforms, Flow *f, const struct MpmListIdDataArgs *cbdata, int list_id) @@ -593,10 +626,15 @@ static InspectionBuffer *GetHttp1HeaderData(DetectEngineThreadCtx *det_ctx, cons if (buffer->initialized) return buffer; - HttpHeaderThreadData *hdr_td = NULL; - HttpHeaderBuffer *buf = - HttpHeaderGetBufferSpace(det_ctx, f, flags, g_keyword_thread_id, &hdr_td); - if (unlikely(buf == NULL)) { + int kw_thread_id; + if (flags & STREAM_TOSERVER) { + kw_thread_id = g_request_header_thread_id; + } else { + kw_thread_id = g_response_header_thread_id; + } + HttpMultiBufHeaderThreadData *hdr_td = + DetectThreadCtxGetGlobalKeywordThreadCtx(det_ctx, kw_thread_id); + if (unlikely(hdr_td == NULL)) { return NULL; } @@ -607,33 +645,53 @@ static InspectionBuffer *GetHttp1HeaderData(DetectEngineThreadCtx *det_ctx, cons } else { headers = tx->response_headers; } - if (cbdata->local_id < htp_table_size(headers)) { - htp_header_t *h = htp_table_get_index(headers, cbdata->local_id, NULL); - size_t size1 = bstr_size(h->name); - size_t size2 = bstr_size(h->value); - size_t b_len = size1 + 2 + size2; - if (b_len > buf->size) { - if (HttpHeaderExpandBuffer(hdr_td, buf, b_len) != 0) { + size_t no_of_headers = htp_table_size(headers); + if (cbdata->local_id == 0) { + // We initialize a big buffer on first item + // Then, we will just use parts of it + hdr_td->len = 0; + if (hdr_td->cap < no_of_headers) { + void *new_buffer = SCRealloc(hdr_td->items, no_of_headers * sizeof(HttpMultiBufItem)); + if (unlikely(new_buffer == NULL)) { return NULL; } + hdr_td->items = new_buffer; + // zeroes the new part of the items + memset(hdr_td->items + hdr_td->cap, 0, + (no_of_headers - hdr_td->cap) * sizeof(HttpMultiBufItem)); + hdr_td->cap = no_of_headers; } - memcpy(buf->buffer, bstr_ptr(h->name), bstr_size(h->name)); - buf->buffer[size1] = ':'; - buf->buffer[size1 + 1] = ' '; - memcpy(buf->buffer + size1 + 2, bstr_ptr(h->value), bstr_size(h->value)); - buf->len = b_len; - } else { - InspectionBufferSetupMultiEmpty(buffer); - return NULL; - } - if (buf->len == 0) { - InspectionBufferSetupMultiEmpty(buffer); - return NULL; + for (size_t i = 0; i < no_of_headers; i++) { + htp_header_t *h = htp_table_get_index(headers, i, NULL); + size_t size1 = bstr_size(h->name); + size_t size2 = bstr_size(h->value); + size_t size = size1 + size2 + 2; + if (hdr_td->items[i].len < size) { + // Use realloc, as this pointer is not freed until HttpMultiBufHeaderThreadDataFree + hdr_td->items[i].buffer = SCRealloc(hdr_td->items[i].buffer, size); + if (unlikely(hdr_td->items[i].buffer == NULL)) { + return NULL; + } + } + memcpy(hdr_td->items[i].buffer, bstr_ptr(h->name), size1); + hdr_td->items[i].buffer[size1] = ':'; + hdr_td->items[i].buffer[size1 + 1] = ' '; + memcpy(hdr_td->items[i].buffer + size1 + 2, bstr_ptr(h->value), size2); + hdr_td->items[i].len = size; + } + hdr_td->len = no_of_headers; } - InspectionBufferSetupMulti(buffer, transforms, buf->buffer, buf->len); - - SCReturnPtr(buffer, "InspectionBuffer"); + // cbdata->local_id is the index of the requested header buffer + // hdr_td->len is the number of header buffers + if (cbdata->local_id < hdr_td->len) { + // we have one valid header buffer + InspectionBufferSetupMulti(buffer, transforms, hdr_td->items[cbdata->local_id].buffer, + hdr_td->items[cbdata->local_id].len); + SCReturnPtr(buffer, "InspectionBuffer"); + } // else there are no more header buffer to get + InspectionBufferSetupMultiEmpty(buffer); + return NULL; } static void PrefilterTxHttp1Header(DetectEngineThreadCtx *det_ctx, const void *pectx, Packet *p, @@ -751,6 +809,8 @@ void DetectHttpRequestHeaderRegister(void) DetectBufferTypeSetDescriptionByName("http_request_header", "HTTP header name and value"); g_http_request_header_buffer_id = DetectBufferTypeGetByName("http_request_header"); DetectBufferTypeSupportsMultiInstance("http_request_header"); + g_request_header_thread_id = DetectRegisterThreadCtxGlobalFuncs("http_request_header", + HttpMultiBufHeaderThreadDataInit, NULL, HttpMultiBufHeaderThreadDataFree); } static int DetectHTTPResponseHeaderSetup(DetectEngineCtx *de_ctx, Signature *s, const char *arg) @@ -786,6 +846,8 @@ void DetectHttpResponseHeaderRegister(void) DetectBufferTypeSetDescriptionByName("http_response_header", "HTTP header name and value"); g_http_response_header_buffer_id = DetectBufferTypeGetByName("http_response_header"); DetectBufferTypeSupportsMultiInstance("http_response_header"); + g_response_header_thread_id = DetectRegisterThreadCtxGlobalFuncs("http_response_header", + HttpMultiBufHeaderThreadDataInit, NULL, HttpMultiBufHeaderThreadDataFree); } /************************************Unittests*********************************/