From: Philippe Antoine Date: Mon, 15 Jul 2024 07:23:06 +0000 (+0200) Subject: detect/integers: harmonize parser return handling X-Git-Tag: suricata-8.0.0-beta1~1014 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=daad7f2d417bb730e51df142fb837d216938089f;p=thirdparty%2Fsuricata.git detect/integers: harmonize parser return handling Ticket: 7172 When parsing an integer for a rule keyword fails, we return error straight away, without bothering to try to free the NULL pointer. On the way, remove some one-line wrapper around DetectUxParse --- diff --git a/src/detect-bsize.c b/src/detect-bsize.c index e95c803b5f..3b544b487d 100644 --- a/src/detect-bsize.c +++ b/src/detect-bsize.c @@ -157,20 +157,6 @@ int DetectBsizeMatch(const SigMatchCtx *ctx, const uint64_t buffer_size, bool eo return 0; } -/** - * \brief This function is used to parse bsize options passed via bsize: keyword - * - * \param bsizestr Pointer to the user provided bsize options - * - * \retval bsized pointer to DetectU64Data on success - * \retval NULL on failure - */ - -static DetectU64Data *DetectBsizeParse(const char *str) -{ - return DetectU64Parse(str); -} - static int SigParseGetMaxBsize(const DetectU64Data *bsz) { switch (bsz->mode) { @@ -207,9 +193,9 @@ static int DetectBsizeSetup (DetectEngineCtx *de_ctx, Signature *s, const char * if (list == DETECT_SM_LIST_NOTSET) SCReturnInt(-1); - DetectU64Data *bsz = DetectBsizeParse(sizestr); + DetectU64Data *bsz = DetectU64Parse(sizestr); if (bsz == NULL) - goto error; + SCReturnInt(-1); if (SigMatchAppendSMToList(de_ctx, s, DETECT_BSIZE, (SigMatchCtx *)bsz, list) == NULL) { goto error; diff --git a/src/detect-dsize.c b/src/detect-dsize.c index c128bde73a..12d4da3e10 100644 --- a/src/detect-dsize.c +++ b/src/detect-dsize.c @@ -121,7 +121,7 @@ static int DetectDsizeSetup (DetectEngineCtx *de_ctx, Signature *s, const char * if (DetectGetLastSMFromLists(s, DETECT_DSIZE, -1)) { SCLogError("Can't use 2 or more dsizes in " "the same sig. Invalidating signature."); - goto error; + return -1; } SCLogDebug("\'%s\'", rawstr); @@ -129,7 +129,7 @@ static int DetectDsizeSetup (DetectEngineCtx *de_ctx, Signature *s, const char * dd = DetectU16Parse(rawstr); if (dd == NULL) { SCLogError("Parsing \'%s\' failed", rawstr); - goto error; + return -1; } /* Okay so far so good, lets get this into a SigMatch @@ -138,7 +138,7 @@ static int DetectDsizeSetup (DetectEngineCtx *de_ctx, Signature *s, const char * de_ctx, s, DETECT_DSIZE, (SigMatchCtx *)dd, DETECT_SM_LIST_MATCH); if (sm == NULL) { rs_detect_u16_free(dd); - goto error; + return -1; } SCLogDebug("dd->arg1 %" PRIu16 ", dd->arg2 %" PRIu16 ", dd->mode %" PRIu8 "", dd->arg1, @@ -152,9 +152,6 @@ static int DetectDsizeSetup (DetectEngineCtx *de_ctx, Signature *s, const char * } return 0; - -error: - return -1; } /** diff --git a/src/detect-filesize.c b/src/detect-filesize.c index c29957d287..f021dd6e5e 100644 --- a/src/detect-filesize.c +++ b/src/detect-filesize.c @@ -122,24 +122,18 @@ static int DetectFilesizeMatch (DetectEngineThreadCtx *det_ctx, Flow *f, static int DetectFilesizeSetup (DetectEngineCtx *de_ctx, Signature *s, const char *str) { SCEnter(); - DetectU64Data *fsd = NULL; - - fsd = DetectU64Parse(str); + DetectU64Data *fsd = DetectU64Parse(str); if (fsd == NULL) - goto error; + SCReturnInt(-1); if (SigMatchAppendSMToList( de_ctx, s, DETECT_FILESIZE, (SigMatchCtx *)fsd, g_file_match_list_id) == NULL) { - goto error; + DetectFilesizeFree(de_ctx, fsd); + SCReturnInt(-1); } s->file_flags |= (FILE_SIG_NEED_FILE|FILE_SIG_NEED_SIZE); SCReturnInt(0); - -error: - if (fsd != NULL) - DetectFilesizeFree(de_ctx, fsd); - SCReturnInt(-1); } /** diff --git a/src/detect-icode.c b/src/detect-icode.c index 1634e4a976..33c48582ae 100644 --- a/src/detect-icode.c +++ b/src/detect-icode.c @@ -120,20 +120,17 @@ static int DetectICodeSetup(DetectEngineCtx *de_ctx, Signature *s, const char *i DetectU8Data *icd = NULL; icd = DetectU8Parse(icodestr); - if (icd == NULL) goto error; + if (icd == NULL) + return -1; if (SigMatchAppendSMToList(de_ctx, s, DETECT_ICODE, (SigMatchCtx *)icd, DETECT_SM_LIST_MATCH) == NULL) { - goto error; + rs_detect_u8_free(icd); + return -1; } s->flags |= SIG_FLAG_REQUIRE_PACKET; return 0; - -error: - if (icd != NULL) - rs_detect_u8_free(icd); - return -1; } /** diff --git a/src/detect-itype.c b/src/detect-itype.c index 42f9144f44..66920d5511 100644 --- a/src/detect-itype.c +++ b/src/detect-itype.c @@ -101,20 +101,6 @@ static int DetectITypeMatch (DetectEngineThreadCtx *det_ctx, Packet *p, return DetectU8Match(pitype, itd); } -/** - * \brief This function is used to parse itype options passed via itype: keyword - * - * \param de_ctx Pointer to the detection engine context - * \param itypestr Pointer to the user provided itype options - * - * \retval itd pointer to DetectU8Data on success - * \retval NULL on failure - */ -static DetectU8Data *DetectITypeParse(DetectEngineCtx *de_ctx, const char *itypestr) -{ - return DetectU8Parse(itypestr); -} - /** * \brief this function is used to add the parsed itype data into the current signature * @@ -130,21 +116,18 @@ static int DetectITypeSetup(DetectEngineCtx *de_ctx, Signature *s, const char *i DetectU8Data *itd = NULL; - itd = DetectITypeParse(de_ctx, itypestr); - if (itd == NULL) goto error; + itd = DetectU8Parse(itypestr); + if (itd == NULL) + return -1; if (SigMatchAppendSMToList(de_ctx, s, DETECT_ITYPE, (SigMatchCtx *)itd, DETECT_SM_LIST_MATCH) == NULL) { - goto error; + DetectITypeFree(de_ctx, itd); + return -1; } s->flags |= SIG_FLAG_REQUIRE_PACKET; return 0; - -error: - if (itd != NULL) - DetectITypeFree(de_ctx, itd); - return -1; } /** @@ -216,7 +199,7 @@ static bool PrefilterITypeIsPrefilterable(const Signature *s) static int DetectITypeParseTest01(void) { DetectU8Data *itd = NULL; - itd = DetectITypeParse(NULL, "8"); + itd = DetectU8Parse("8"); FAIL_IF_NULL(itd); FAIL_IF_NOT(itd->arg1 == 8); FAIL_IF_NOT(itd->mode == DETECT_UINT_EQ); @@ -232,7 +215,7 @@ static int DetectITypeParseTest01(void) static int DetectITypeParseTest02(void) { DetectU8Data *itd = NULL; - itd = DetectITypeParse(NULL, ">8"); + itd = DetectU8Parse(">8"); FAIL_IF_NULL(itd); FAIL_IF_NOT(itd->arg1 == 8); FAIL_IF_NOT(itd->mode == DETECT_UINT_GT); @@ -248,7 +231,7 @@ static int DetectITypeParseTest02(void) static int DetectITypeParseTest03(void) { DetectU8Data *itd = NULL; - itd = DetectITypeParse(NULL, "<8"); + itd = DetectU8Parse("<8"); FAIL_IF_NULL(itd); FAIL_IF_NOT(itd->arg1 == 8); FAIL_IF_NOT(itd->mode == DETECT_UINT_LT); @@ -264,7 +247,7 @@ static int DetectITypeParseTest03(void) static int DetectITypeParseTest04(void) { DetectU8Data *itd = NULL; - itd = DetectITypeParse(NULL, "8<>20"); + itd = DetectU8Parse("8<>20"); FAIL_IF_NULL(itd); FAIL_IF_NOT(itd->arg1 == 8); FAIL_IF_NOT(itd->arg2 == 20); @@ -281,7 +264,7 @@ static int DetectITypeParseTest04(void) static int DetectITypeParseTest05(void) { DetectU8Data *itd = NULL; - itd = DetectITypeParse(NULL, " 8 "); + itd = DetectU8Parse(" 8 "); FAIL_IF_NULL(itd); FAIL_IF_NOT(itd->arg1 == 8); FAIL_IF_NOT(itd->mode == DETECT_UINT_EQ); @@ -297,7 +280,7 @@ static int DetectITypeParseTest05(void) static int DetectITypeParseTest06(void) { DetectU8Data *itd = NULL; - itd = DetectITypeParse(NULL, " > 8 "); + itd = DetectU8Parse(" > 8 "); FAIL_IF_NULL(itd); FAIL_IF_NOT(itd->arg1 == 8); FAIL_IF_NOT(itd->mode == DETECT_UINT_GT); @@ -313,7 +296,7 @@ static int DetectITypeParseTest06(void) static int DetectITypeParseTest07(void) { DetectU8Data *itd = NULL; - itd = DetectITypeParse(NULL, " 8 <> 20 "); + itd = DetectU8Parse(" 8 <> 20 "); FAIL_IF_NULL(itd); FAIL_IF_NOT(itd->arg1 == 8); FAIL_IF_NOT(itd->arg2 == 20); @@ -329,7 +312,7 @@ static int DetectITypeParseTest07(void) static int DetectITypeParseTest08(void) { DetectU8Data *itd = NULL; - itd = DetectITypeParse(NULL, "> 8 <> 20"); + itd = DetectU8Parse("> 8 <> 20"); FAIL_IF_NOT_NULL(itd); PASS; diff --git a/src/detect-rfb-sectype.c b/src/detect-rfb-sectype.c index c9afa8b46c..d3758d3ded 100644 --- a/src/detect-rfb-sectype.c +++ b/src/detect-rfb-sectype.c @@ -90,20 +90,6 @@ static int DetectRfbSectypeMatch (DetectEngineThreadCtx *det_ctx, SCReturnInt(0); } -/** - * \internal - * \brief Function to parse options passed via rfb.sectype keywords. - * - * \param rawstr Pointer to the user provided options. - * - * \retval dd pointer to DetectU32Data on success. - * \retval NULL on failure. - */ -static DetectU32Data *DetectRfbSectypeParse(const char *rawstr) -{ - return DetectU32Parse(rawstr); -} - /** * \brief Function to add the parsed RFB security type field into the current signature. * @@ -119,10 +105,10 @@ static int DetectRfbSectypeSetup (DetectEngineCtx *de_ctx, Signature *s, const c if (DetectSignatureSetAppProto(s, ALPROTO_RFB) != 0) return -1; - DetectU32Data *dd = DetectRfbSectypeParse(rawstr); + DetectU32Data *dd = DetectU32Parse(rawstr); if (dd == NULL) { SCLogError("Parsing \'%s\' failed", rawstr); - goto error; + return -1; } /* okay so far so good, lets get this into a SigMatch diff --git a/src/tests/detect-bsize.c b/src/tests/detect-bsize.c index 2fcd656589..f0b13a8944 100644 --- a/src/tests/detect-bsize.c +++ b/src/tests/detect-bsize.c @@ -19,7 +19,7 @@ #define TEST_OK(str, m, lo, hi) \ { \ - DetectU64Data *bsz = DetectBsizeParse((str)); \ + DetectU64Data *bsz = DetectU64Parse((str)); \ FAIL_IF_NULL(bsz); \ FAIL_IF_NOT(bsz->mode == (m)); \ DetectBsizeFree(NULL, bsz); \ @@ -27,7 +27,7 @@ } #define TEST_FAIL(str) \ { \ - DetectU64Data *bsz = DetectBsizeParse((str)); \ + DetectU64Data *bsz = DetectU64Parse((str)); \ FAIL_IF_NOT_NULL(bsz); \ }