From: Jeff Lucovsky Date: Fri, 9 Jun 2023 14:32:18 +0000 (-0400) Subject: detect/bytejump: Allow nbytes to be a variable X-Git-Tag: suricata-7.0.0~49 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3f118188e9ae09efc026b43f3b76ddfde0aed7d5;p=thirdparty%2Fsuricata.git detect/bytejump: Allow nbytes to be a variable Issue: 6105 This commit adds the ability for nbytes to be a variable when used with the byte_jump keyword. --- diff --git a/src/detect-bytejump.c b/src/detect-bytejump.c index a15eb6145a..4054f04934 100644 --- a/src/detect-bytejump.c +++ b/src/detect-bytejump.c @@ -63,7 +63,8 @@ static DetectParseRegex parse_regex; static int DetectBytejumpMatch(DetectEngineThreadCtx *det_ctx, Packet *p, const Signature *s, const SigMatchCtx *ctx); -static DetectBytejumpData *DetectBytejumpParse(DetectEngineCtx *de_ctx, const char *optstr, char **offset); +static DetectBytejumpData *DetectBytejumpParse( + DetectEngineCtx *de_ctx, const char *optstr, char **nbytes, char **offset); static int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, const char *optstr); static void DetectBytejumpFree(DetectEngineCtx*, void *ptr); #ifdef UNITTESTS @@ -84,6 +85,47 @@ void DetectBytejumpRegister (void) DetectSetupParseRegexes(PARSE_REGEX, &parse_regex); } +/* 23 - This is the largest string (octal, with a zero prefix) that + * will not overflow uint64_t. The only way this length + * could be over 23 and still not overflow is if it were zero + * prefixed and we only support 1 byte of zero prefix for octal. + * + * "01777777777777777777777" = 0xffffffffffffffff + * + * 8 - Without string, the maximum byte extract count is 8. + */ +static inline bool DetectBytejumpValidateNbytesOnly(const DetectBytejumpData *data, int32_t nbytes) +{ + return (data->flags & DETECT_BYTEJUMP_STRING && nbytes <= 23) || (nbytes <= 8); +} + +static bool DetectBytejumpValidateNbytes(const DetectBytejumpData *data, int32_t nbytes) +{ + if (!DetectBytejumpValidateNbytesOnly(data, nbytes)) { + if (data->flags & DETECT_BYTEJUMP_STRING) { + /* 23 - This is the largest string (octal, with a zero prefix) that + * will not overflow uint64_t. The only way this length + * could be over 23 and still not overflow is if it were zero + * prefixed and we only support 1 byte of zero prefix for octal. + * + * "01777777777777777777777" = 0xffffffffffffffff + */ + if (nbytes > 23) { + SCLogError("Cannot test more than 23 bytes " + "with \"string\""); + } + } else { + if (nbytes > 8) { + SCLogError("Cannot test more than 8 bytes " + "without \"string\""); + } + } + return false; + } + + return true; +} + /** \brief Byte jump match function * \param det_ctx thread detect engine ctx * \param s signature @@ -94,8 +136,8 @@ void DetectBytejumpRegister (void) * \retval 0 no match */ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s, - const SigMatchCtx *ctx, const uint8_t *payload, uint32_t payload_len, - uint16_t flags, int32_t offset) + const SigMatchCtx *ctx, const uint8_t *payload, uint32_t payload_len, uint16_t flags, + int32_t nbytes, int32_t offset) { SCEnter(); @@ -109,6 +151,20 @@ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s, SCReturnInt(0); } + /* Validate the number of bytes we are testing + * If the validation is successful, we know that + * it contains a value <= 23. Thus, we can + * safely cast it when extracting bytes + */ + if (data->flags & DETECT_BYTEJUMP_NBYTES_VAR) { + if (!DetectBytejumpValidateNbytesOnly(data, nbytes)) { + SCLogDebug("Invalid byte_jump nbytes " + "seen in byte_jump - %d", + nbytes); + SCReturnInt(0); + } + } + /* Calculate the ptr value for the bytejump and length remaining in * the packet from that point. */ @@ -130,29 +186,26 @@ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s, } /* Verify the to-be-extracted data is within the packet */ - if (ptr < payload || data->nbytes > len) { + if (ptr < payload || nbytes > len) { SCLogDebug("Data not within payload " - "pkt=%p, ptr=%p, len=%d, nbytes=%d", - payload, ptr, len, data->nbytes); + "pkt=%p, ptr=%p, len=%d, nbytes=%d", + payload, ptr, len, nbytes); SCReturnInt(0); } /* Extract the byte data */ if (flags & DETECT_BYTEJUMP_STRING) { - extbytes = ByteExtractStringUint64(&val, data->base, - data->nbytes, (const char *)ptr); + extbytes = ByteExtractStringUint64(&val, data->base, nbytes, (const char *)ptr); if(extbytes <= 0) { - SCLogDebug("error extracting %d bytes of string data: %d", - data->nbytes, extbytes); + SCLogDebug("error extracting %d bytes of string data: %d", nbytes, extbytes); SCReturnInt(0); } } else { int endianness = (flags & DETECT_BYTEJUMP_LITTLE) ? BYTE_LITTLE_ENDIAN : BYTE_BIG_ENDIAN; - extbytes = ByteExtractUint64(&val, endianness, data->nbytes, ptr); - if (extbytes != data->nbytes) { - SCLogDebug("error extracting %d bytes of numeric data: %d", - data->nbytes, extbytes); + extbytes = ByteExtractUint64(&val, endianness, (uint16_t)nbytes, ptr); + if (extbytes != nbytes) { + SCLogDebug("error extracting %d bytes of numeric data: %d", nbytes, extbytes); SCReturnInt(0); } } @@ -179,7 +232,6 @@ int DetectBytejumpDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s, SCLogDebug("NEWVAL: ptr %p + %" PRIu64, ptr, val); } - /* Validate that the jump location is still in the packet * \todo Should this validate it is still in the *payload*? */ @@ -315,7 +367,8 @@ static int DetectBytejumpMatch(DetectEngineThreadCtx *det_ctx, return 1; } -static DetectBytejumpData *DetectBytejumpParse(DetectEngineCtx *de_ctx, const char *optstr, char **offset) +static DetectBytejumpData *DetectBytejumpParse( + DetectEngineCtx *de_ctx, const char *optstr, char **nbytes_str, char **offset) { DetectBytejumpData *data = NULL; char args[10][64]; @@ -323,7 +376,7 @@ static DetectBytejumpData *DetectBytejumpParse(DetectEngineCtx *de_ctx, const ch size_t pcre2len; int numargs = 0; int i = 0; - uint32_t nbytes; + uint32_t nbytes = 0; char *str_ptr; char *end_ptr; @@ -396,9 +449,22 @@ static DetectBytejumpData *DetectBytejumpParse(DetectEngineCtx *de_ctx, const ch */ /* Number of bytes */ - if (StringParseUint32(&nbytes, 10, (uint16_t)strlen(args[0]), args[0]) <= 0) { - SCLogError("Malformed number of bytes: %s", optstr); - goto error; + if (args[0][0] != '-' && isalpha((unsigned char)args[0][0])) { + if (nbytes_str == NULL) { + SCLogError("byte_jump supplied with " + "var name for nbytes. \"value\" argument supplied to " + "this function has to be non-NULL"); + goto error; + } + *nbytes_str = SCStrdup(args[0]); + if (*nbytes_str == NULL) + goto error; + data->flags |= DETECT_BYTEJUMP_NBYTES_VAR; + } else { + if (StringParseUint32(&nbytes, 10, (uint16_t)strlen(args[0]), args[0]) <= 0) { + SCLogError("Malformed number of bytes: %s", optstr); + goto error; + } } /* Offset */ @@ -446,8 +512,8 @@ static DetectBytejumpData *DetectBytejumpParse(DetectEngineCtx *de_ctx, const ch } else if (strcasecmp("align", args[i]) == 0) { data->flags |= DETECT_BYTEJUMP_ALIGN; } else if (strncasecmp("multiplier ", args[i], 11) == 0) { - if (StringParseUint32( - &data->multiplier, 10, (uint16_t)strlen(args[i]) - 11, args[i] + 11) <= 0) { + if (StringParseU16RangeCheck(&data->multiplier, 10, (uint16_t)strlen(args[i]) - 11, + args[i] + 11, 1, 65535) <= 0) { SCLogError("Malformed multiplier: %s", optstr); goto error; } @@ -471,27 +537,15 @@ static DetectBytejumpData *DetectBytejumpParse(DetectEngineCtx *de_ctx, const ch goto error; } - if (data->flags & DETECT_BYTEJUMP_STRING) { - /* 23 - This is the largest string (octal, with a zero prefix) that - * will not overflow uint64_t. The only way this length - * could be over 23 and still not overflow is if it were zero - * prefixed and we only support 1 byte of zero prefix for octal. - * - * "01777777777777777777777" = 0xffffffffffffffff - */ - if (nbytes > 23) { - SCLogError("Cannot test more than 23 bytes " - "with \"string\": %s", - optstr); - goto error; - } - } else { - if (nbytes > 8) { - SCLogError("Cannot test more than 8 bytes " - "without \"string\": %s", - optstr); + if (!(data->flags & DETECT_BYTEJUMP_NBYTES_VAR)) { + if (!DetectBytejumpValidateNbytes(data, nbytes)) { goto error; } + + /* This is max 23 so it will fit in a byte (see validation function) */ + data->nbytes = (uint8_t)nbytes; + } + if (!(data->flags & DETECT_BYTEJUMP_STRING)) { if (data->base != DETECT_BYTEJUMP_BASE_UNSET) { SCLogError("Cannot use a base " "without \"string\": %s", @@ -500,9 +554,6 @@ static DetectBytejumpData *DetectBytejumpParse(DetectEngineCtx *de_ctx, const ch } } - /* This is max 23 so it will fit in a byte (see above) */ - data->nbytes = (uint8_t)nbytes; - return data; error: @@ -510,6 +561,10 @@ error: SCFree(*offset); *offset = NULL; } + if (nbytes_str != NULL && *nbytes_str != NULL) { + SCFree(*nbytes_str); + *nbytes_str = NULL; + } if (data != NULL) DetectBytejumpFree(de_ctx, data); return NULL; @@ -521,9 +576,10 @@ static int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, const char SigMatch *prev_pm = NULL; DetectBytejumpData *data = NULL; char *offset = NULL; + char *nbytes = NULL; int ret = -1; - data = DetectBytejumpParse(de_ctx, optstr, &offset); + data = DetectBytejumpParse(de_ctx, optstr, &nbytes, &offset); if (data == NULL) goto error; @@ -589,6 +645,19 @@ static int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, const char } } + if (nbytes != NULL) { + DetectByteIndexType index; + if (!DetectByteRetrieveSMVar(nbytes, s, &index)) { + SCLogError("Unknown byte_extract var " + "seen in byte_jump - %s", + nbytes); + goto error; + } + data->nbytes = index; + SCFree(nbytes); + nbytes = NULL; + } + if (offset != NULL) { DetectByteIndexType index; if (!DetectByteRetrieveSMVar(offset, s, &index)) { @@ -627,7 +696,11 @@ static int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, const char okay: ret = 0; return ret; + error: + if (nbytes != NULL) { + SCFree(nbytes); + } if (offset != NULL) { SCFree(offset); } @@ -664,7 +737,7 @@ static int DetectBytejumpTestParse01(void) { int result = 0; DetectBytejumpData *data = NULL; - data = DetectBytejumpParse(NULL, "4,0", NULL); + data = DetectBytejumpParse(NULL, "4,0", NULL, NULL); if (data != NULL) { DetectBytejumpFree(NULL, data); result = 1; @@ -680,7 +753,7 @@ static int DetectBytejumpTestParse02(void) { int result = 0; DetectBytejumpData *data = NULL; - data = DetectBytejumpParse(NULL, "4, 0", NULL); + data = DetectBytejumpParse(NULL, "4, 0", NULL, NULL); if (data != NULL) { if ( (data->nbytes == 4) && (data->offset == 0) @@ -704,8 +777,10 @@ static int DetectBytejumpTestParse03(void) { int result = 0; DetectBytejumpData *data = NULL; - data = DetectBytejumpParse(NULL, " 4,0 , relative , little, string, " - "dec, align, from_beginning", NULL); + data = DetectBytejumpParse(NULL, + " 4,0 , relative , little, string, " + "dec, align, from_beginning", + NULL, NULL); if (data != NULL) { if ( (data->nbytes == 4) && (data->offset == 0) @@ -736,9 +811,11 @@ static int DetectBytejumpTestParse04(void) { int result = 0; DetectBytejumpData *data = NULL; - data = DetectBytejumpParse(NULL, " 4,0 , relative , little, string, " - "dec, align, from_beginning , " - "multiplier 2 , post_offset -16 ", NULL); + data = DetectBytejumpParse(NULL, + " 4,0 , relative , little, string, " + "dec, align, from_beginning , " + "multiplier 2 , post_offset -16 ", + NULL, NULL); if (data != NULL) { if ( (data->nbytes == 4) && (data->offset == 0) @@ -766,8 +843,10 @@ static int DetectBytejumpTestParse05(void) { int result = 0; DetectBytejumpData *data = NULL; - data = DetectBytejumpParse(NULL, " 4,0 , relative , little, dec, " - "align, from_beginning", NULL); + data = DetectBytejumpParse(NULL, + " 4,0 , relative , little, dec, " + "align, from_beginning", + NULL, NULL); if (data == NULL) { result = 1; } @@ -782,7 +861,7 @@ static int DetectBytejumpTestParse06(void) { int result = 0; DetectBytejumpData *data = NULL; - data = DetectBytejumpParse(NULL, "9, 0", NULL); + data = DetectBytejumpParse(NULL, "9, 0", NULL, NULL); if (data == NULL) { result = 1; } @@ -797,7 +876,7 @@ static int DetectBytejumpTestParse07(void) { int result = 0; DetectBytejumpData *data = NULL; - data = DetectBytejumpParse(NULL, "24, 0, string, dec", NULL); + data = DetectBytejumpParse(NULL, "24, 0, string, dec", NULL, NULL); if (data == NULL) { result = 1; } @@ -812,7 +891,7 @@ static int DetectBytejumpTestParse08(void) { int result = 0; DetectBytejumpData *data = NULL; - data = DetectBytejumpParse(NULL, "4, 0xffffffffffffffff", NULL); + data = DetectBytejumpParse(NULL, "4, 0xffffffffffffffff", NULL, NULL); if (data == NULL) { result = 1; } @@ -1067,7 +1146,9 @@ static int DetectBytejumpTestParse12(void) static int DetectBytejumpTestParse13(void) { DetectBytejumpData *data = DetectBytejumpParse(NULL, - " 4,0 , relative , little, string, dec, " "align, from_end", NULL); + " 4,0 , relative , little, string, dec, " + "align, from_end", + NULL, NULL); FAIL_IF_NULL(data); FAIL_IF_NOT(data->flags & DETECT_BYTEJUMP_END); @@ -1080,7 +1161,8 @@ static int DetectBytejumpTestParse14(void) { DetectBytejumpData *data = DetectBytejumpParse(NULL, " 4,0 , relative , little, string, dec, " - "align, from_beginning, from_end", NULL); + "align, from_beginning, from_end", + NULL, NULL); FAIL_IF_NOT_NULL(data); diff --git a/src/detect-bytejump.h b/src/detect-bytejump.h index 91242fe0c8..e1850ec770 100644 --- a/src/detect-bytejump.h +++ b/src/detect-bytejump.h @@ -40,14 +40,15 @@ #define DETECT_BYTEJUMP_DCE BIT_U16(6) /**< "dce" enabled */ #define DETECT_BYTEJUMP_OFFSET_BE BIT_U16(7) /**< "byte extract" enabled */ #define DETECT_BYTEJUMP_END BIT_U16(8) /**< "from_end" jump */ +#define DETECT_BYTEJUMP_NBYTES_VAR BIT_U16(9) /**< nbytes string*/ typedef struct DetectBytejumpData_ { uint8_t nbytes; /**< Number of bytes to compare */ uint8_t base; /**< String value base (oct|dec|hex) */ uint16_t flags; /**< Flags (big|little|relative|string) */ - uint32_t multiplier; /**< Multiplier for nbytes (multiplier n)*/ int32_t offset; /**< Offset in payload to extract value */ int32_t post_offset; /**< Offset to adjust post-jump */ + uint16_t multiplier; /**< Multiplier for nbytes (multiplier n)*/ } DetectBytejumpData; /* prototypes */ @@ -77,7 +78,7 @@ void DetectBytejumpRegister (void); * error as a match. */ int DetectBytejumpDoMatch(DetectEngineThreadCtx *, const Signature *, const SigMatchCtx *, - const uint8_t *, uint32_t, uint16_t, int32_t); + const uint8_t *, uint32_t, uint16_t, int32_t, int32_t); #endif /* __DETECT_BYTEJUMP_H__ */ diff --git a/src/detect-engine-content-inspection.c b/src/detect-engine-content-inspection.c index c9df628808..0ca8b3ee39 100644 --- a/src/detect-engine-content-inspection.c +++ b/src/detect-engine-content-inspection.c @@ -513,11 +513,18 @@ uint8_t DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThrea DetectBytejumpData *bjd = (DetectBytejumpData *)smd->ctx; uint16_t bjflags = bjd->flags; int32_t offset = bjd->offset; + int32_t nbytes; if (bjflags & DETECT_CONTENT_OFFSET_VAR) { offset = det_ctx->byte_values[offset]; } + if (bjflags & DETECT_BYTEJUMP_NBYTES_VAR) { + nbytes = det_ctx->byte_values[bjd->nbytes]; + } else { + nbytes = bjd->nbytes; + } + /* if we have dce enabled we will have to use the endianness * specified by the dce header */ if (bjflags & DETECT_BYTEJUMP_DCE) { @@ -527,8 +534,8 @@ uint8_t DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThrea DETECT_BYTEJUMP_LITTLE: 0); } - if (DetectBytejumpDoMatch(det_ctx, s, smd->ctx, buffer, buffer_len, - bjflags, offset) != 1) { + if (DetectBytejumpDoMatch( + det_ctx, s, smd->ctx, buffer, buffer_len, bjflags, nbytes, offset) != 1) { goto no_match; }