]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect/integers: harmonize parser return handling 11506/head
authorPhilippe Antoine <pantoine@oisf.net>
Mon, 15 Jul 2024 07:23:06 +0000 (09:23 +0200)
committerVictor Julien <victor@inliniac.net>
Mon, 15 Jul 2024 12:25:36 +0000 (14:25 +0200)
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

src/detect-bsize.c
src/detect-dsize.c
src/detect-filesize.c
src/detect-icode.c
src/detect-itype.c
src/detect-rfb-sectype.c
src/tests/detect-bsize.c

index e95c803b5f3d4ba8096c3a009bae1893d9218ae3..3b544b487d255028381f66d9d3bd366770872421 100644 (file)
@@ -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;
index c128bde73a4d209394353ed95773f4027a6174ed..12d4da3e10651bcfe029b3c33eb61e25dd62ad83 100644 (file)
@@ -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;
 }
 
 /**
index c29957d2870b40878c028261ce1dedca3e09f80c..f021dd6e5edaac0ba241f85c321b04a4e5b07a58 100644 (file)
@@ -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);
 }
 
 /**
index 1634e4a9767078d8db20b2492a9e153e41c407a3..33c48582ae1c394067dc53d4eb0408943cb52107 100644 (file)
@@ -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;
 }
 
 /**
index 42f9144f44690140aafc8fc42f1210af416d2e70..66920d5511c133f8eeb49a589c028ad6a24ec105 100644 (file)
@@ -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;
index c9afa8b46c0f54f088ff0156b855e3b1680d17e9..d3758d3deda71347df5ad38cf5ec15e70db5155d 100644 (file)
@@ -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
index 2fcd656589907f9a9d92650ffec5195705cbd595..f0b13a8944a6ea2ba8eb9d036cf44b1f641bdd66 100644 (file)
@@ -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);                                                                     \
     }