From 32d0bd2bbb4d486623dec85a94952fde2515f2f0 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 17 Dec 2024 15:06:25 +0100 Subject: [PATCH] detect: limit base64_decode `bytes` to 64KiB Ticket: 7613 Avoids potential large per-thread memory allocation. A buffer with the size of the largest decode_base64 buffer size setting would be allocated per thread. As this was a u32, it could mean a per-thread 4GiB memory allocation. 64KiB was already the built-in default for cases where bytes size wasn't specified. --- doc/userguide/rules/base64-keywords.rst | 1 + src/detect-base64-decode.c | 15 ++++++--------- src/detect.h | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/doc/userguide/rules/base64-keywords.rst b/doc/userguide/rules/base64-keywords.rst index 9e4f942e46..ee19c8a03a 100644 --- a/doc/userguide/rules/base64-keywords.rst +++ b/doc/userguide/rules/base64-keywords.rst @@ -17,6 +17,7 @@ Syntax:: base64_decode:bytes , offset , relative; The ``bytes`` option specifies how many bytes Suricata should decode and make available for base64_data. +This number is limited to 64KiB. The decoding will stop at the end of the buffer. The ``offset`` option specifies how many bytes Suricata should skip before decoding. diff --git a/src/detect-base64-decode.c b/src/detect-base64-decode.c index 4a2219a3ce..35d79c9004 100644 --- a/src/detect-base64-decode.c +++ b/src/detect-base64-decode.c @@ -28,7 +28,7 @@ #define BASE64_DECODE_MAX 65535 typedef struct DetectBase64Decode_ { - uint32_t bytes; + uint16_t bytes; uint32_t offset; uint8_t relative; } DetectBase64Decode; @@ -110,8 +110,8 @@ int DetectBase64DecodeDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s return det_ctx->base64_decoded_len > 0; } -static int DetectBase64DecodeParse(const char *str, uint32_t *bytes, - uint32_t *offset, uint8_t *relative) +static int DetectBase64DecodeParse( + const char *str, uint16_t *bytes, uint32_t *offset, uint8_t *relative) { const char *bytes_str = NULL; const char *offset_str = NULL; @@ -131,7 +131,7 @@ static int DetectBase64DecodeParse(const char *str, uint32_t *bytes, if (pcre_rc >= 3) { if (pcre2_substring_get_bynumber(match, 2, (PCRE2_UCHAR8 **)&bytes_str, &pcre2_len) == 0) { - if (StringParseUint32(bytes, 10, 0, bytes_str) <= 0) { + if (StringParseUint16(bytes, 10, 0, bytes_str) <= 0) { SCLogError("Bad value for bytes: \"%s\"", bytes_str); goto error; } @@ -185,7 +185,7 @@ error: static int DetectBase64DecodeSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) { - uint32_t bytes = 0; + uint16_t bytes = 0; uint32_t offset = 0; uint8_t relative = 0; DetectBase64Decode *data = NULL; @@ -233,9 +233,6 @@ static int DetectBase64DecodeSetup(DetectEngineCtx *de_ctx, Signature *s, data->bytes = BASE64_DECODE_MAX; } if (data->bytes > de_ctx->base64_decode_max_len) { -#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION - data->bytes = BASE64_DECODE_MAX; -#endif de_ctx->base64_decode_max_len = data->bytes; } @@ -267,7 +264,7 @@ static int g_http_header_buffer_id = 0; static int DetectBase64TestDecodeParse(void) { int retval = 0; - uint32_t bytes = 0; + uint16_t bytes = 0; uint32_t offset = 0; uint8_t relative = 0; diff --git a/src/detect.h b/src/detect.h index cea44e7a1b..90507ff031 100644 --- a/src/detect.h +++ b/src/detect.h @@ -943,7 +943,7 @@ typedef struct DetectEngineCtx_ { struct SigGroupHead_ *decoder_event_sgh; /* Maximum size of the buffer for decoded base64 data. */ - uint32_t base64_decode_max_len; + uint16_t base64_decode_max_len; /** Store rule file and line so that parsers can use them in errors. */ int rule_line; -- 2.47.2