detect/parse-port: fix whitespaces bug

Updated by Victor Julien to:
- accept but strip leading whitespaces
- update tests
- cleanup tests

src/detect-engine-port.c

@@ -1284,14 +1284,18 @@ DetectPort *PortParse(const char *str)
 {
     char *port2 = NULL;
     char portstr[16];
+
+    /* strip leading spaces */
+    while (isspace(*str))
+        str++;
+    if (strlen(str) >= 16)
+        return NULL;
     strlcpy(portstr, str, sizeof(portstr));
 
     DetectPort *dp = DetectPortInit();
     if (dp == NULL)
         goto error;
 
-    /* XXX better input validation */
-
     /* we dup so we can put a nul-termination in it later */
     char *port = portstr;
 
@@ -2466,6 +2470,82 @@ static int PortTestMatchDoubleNegation(void)
     return result;
 }
 
+// Test that negation is successfully parsed with whitespace for port strings of
+// length < 16
+static int DetectPortParseDoTest(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    FAIL_IF_NULL(de_ctx);
+    DetectPort *head = NULL;
+    DetectPort *nhead = NULL;
+    const char *str = "[30:50, !45]";
+    int r = DetectPortParseDo(de_ctx, &head, &nhead, str, 0, NULL, 0);
+
+    // Assertions
+    FAIL_IF_NULL(head);
+    FAIL_IF_NULL(nhead);
+    FAIL_IF(r < 0);
+    FAIL_IF(head->port != 30);
+    FAIL_IF(head->port2 != 50);
+    FAIL_IF(nhead->port != 45);
+    FAIL_IF(nhead->port2 != 45);
+    DetectPortCleanupList(NULL, head);
+    DetectPortCleanupList(NULL, nhead);
+    PASS;
+}
+
+static int DetectPortParseDoTest2(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    FAIL_IF_NULL(de_ctx);
+    DetectPort *head = NULL;
+    DetectPort *nhead = NULL;
+    const char *str = "[30:50,              !45]";
+    int r = DetectPortParseDo(de_ctx, &head, &nhead, str, 0, NULL, 0);
+    FAIL_IF(r < 0);
+    DetectPortCleanupList(NULL, head);
+    DetectPortCleanupList(NULL, nhead);
+    PASS;
+}
+
+// Verifies correct parsing when negation port string length < 16
+static int PortParseTestLessThan14Spaces(void)
+{
+    const char *str = "       45";
+    DetectPort *dp = PortParse(str);
+    FAIL_IF_NULL(dp);
+    FAIL_IF(dp->port != 45);
+    FAIL_IF(dp->port2 != 45);
+    DetectPortFree(NULL, dp);
+    PASS;
+}
+
+// Verifies NULL returned when negation port string length == 16
+static int PortParseTest14Spaces(void)
+{
+    const char *str = "              45";
+    DetectPort *dp = PortParse(str);
+    FAIL_IF_NULL(dp);
+    FAIL_IF_NULL(dp);
+    FAIL_IF(dp->port != 45);
+    FAIL_IF(dp->port2 != 45);
+    DetectPortFree(NULL, dp);
+    PASS;
+}
+
+// Verifies NULL returned when negation port string length >= 16
+static int PortParseTestMoreThan14Spaces(void)
+{
+    const char *str = "                                   45";
+    DetectPort *dp = PortParse(str);
+    FAIL_IF_NULL(dp);
+    FAIL_IF_NULL(dp);
+    FAIL_IF(dp->port != 45);
+    FAIL_IF(dp->port2 != 45);
+    DetectPortFree(NULL, dp);
+    PASS;
+}
+
 void DetectPortTests(void)
 {
     UtRegisterTest("PortTestParse01", PortTestParse01);
@@ -2510,6 +2590,11 @@ void DetectPortTests(void)
     UtRegisterTest("PortTestMatchReal18", PortTestMatchReal18);
     UtRegisterTest("PortTestMatchReal19", PortTestMatchReal19);
     UtRegisterTest("PortTestMatchDoubleNegation", PortTestMatchDoubleNegation);
+    UtRegisterTest("DetectPortParseDoTest", DetectPortParseDoTest);
+    UtRegisterTest("DetectPortParseDoTest2", DetectPortParseDoTest2);
+    UtRegisterTest("PortParseTestLessThan14Spaces", PortParseTestLessThan14Spaces);
+    UtRegisterTest("PortParseTest14Spaces", PortParseTest14Spaces);
+    UtRegisterTest("PortParseTestMoreThan14Spaces", PortParseTestMoreThan14Spaces);
 }
 
 #endif /* UNITTESTS */

src/tests/detect-parse.c

@@ -1,5 +1,4 @@
-
-/* Copyright (C) 2007-2019 Open Information Security Foundation
+/* Copyright (C) 2007-2022 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -16,8 +15,12 @@
  * 02110-1301, USA.
  */
 
+#include "../detect.h"
 #include "../detect-parse.h"
+#include "../detect-engine-port.h"
 #include "../util-unittest.h"
+#include "util-debug.h"
+#include "util-error.h"
 
 /**
  * \test DetectParseTest01 is a regression test against a memory leak
@@ -56,6 +59,87 @@ static int DetectParseTestNoOpt(void)
     PASS;
 }
 
+static int SigParseTestNegatationNoWhitespace(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    FAIL_IF_NULL(de_ctx);
+    Signature *s = DetectEngineAppendSig(de_ctx,
+            "alert http any [30:50,!45] -> any [30:50,!45] (msg:\"sid 2 version 0\"; "
+            "content:\"dummy2\"; sid:2;)");
+    FAIL_IF_NULL(s);
+    FAIL_IF_NULL(s->sp);
+    FAIL_IF_NULL(s->dp);
+    FAIL_IF_NOT(s->sp->port == 30);
+    FAIL_IF_NOT(s->sp->port2 == 44);
+    FAIL_IF_NULL(s->sp->next);
+    FAIL_IF_NOT(s->sp->next->port == 46);
+    FAIL_IF_NOT(s->sp->next->port2 == 50);
+    FAIL_IF_NOT_NULL(s->sp->next->next);
+    DetectEngineCtxFree(de_ctx);
+    PASS;
+}
+
+// // Tests proper Signature is parsed from portstring length < 16 ie [30:50, !45]
+static int SigParseTestWhitespaceLessThan14(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    FAIL_IF_NULL(de_ctx);
+    Signature *s = DetectEngineAppendSig(de_ctx,
+            "alert http any [30:50, !45] -> any [30:50,!45] (msg:\"sid 2 version 0\"; "
+            "content:\"dummy2\"; sid:2;)");
+    FAIL_IF_NULL(s);
+    FAIL_IF_NULL(s->sp);
+    FAIL_IF_NULL(s->dp);
+    FAIL_IF_NOT(s->sp->port == 30);
+    FAIL_IF_NOT(s->sp->port2 == 44);
+    FAIL_IF_NULL(s->sp->next);
+    FAIL_IF_NOT(s->sp->next->port == 46);
+    FAIL_IF_NOT(s->sp->next->port2 == 50);
+    FAIL_IF_NOT_NULL(s->sp->next->next);
+    DetectEngineCtxFree(de_ctx);
+    PASS;
+}
+
+static int SigParseTestWhitespace14Spaces(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    FAIL_IF_NULL(de_ctx);
+    Signature *s = DetectEngineAppendSig(de_ctx,
+            "alert http any [30:50,              !45] -> any [30:50,!45] (msg:\"sid 2 "
+            "version 0\"; content:\"dummy2\"; sid:2;)");
+    FAIL_IF_NULL(s);
+    FAIL_IF_NULL(s->sp);
+    FAIL_IF_NULL(s->dp);
+    FAIL_IF_NOT(s->sp->port == 30);
+    FAIL_IF_NOT(s->sp->port2 == 44);
+    FAIL_IF_NULL(s->sp->next);
+    FAIL_IF_NOT(s->sp->next->port == 46);
+    FAIL_IF_NOT(s->sp->next->port2 == 50);
+    FAIL_IF_NOT_NULL(s->sp->next->next);
+    DetectEngineCtxFree(de_ctx);
+    PASS;
+}
+
+static int SigParseTestWhitespaceMoreThan14(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    FAIL_IF_NULL(de_ctx);
+    Signature *s = DetectEngineAppendSig(de_ctx,
+            "alert http any [30:50,                          !45] -> any [30:50,!45] "
+            "(msg:\"sid 2 version 0\"; content:\"dummy2\"; sid:2;)");
+    FAIL_IF_NULL(s);
+    FAIL_IF_NULL(s->sp);
+    FAIL_IF_NULL(s->dp);
+    FAIL_IF_NOT(s->sp->port == 30);
+    FAIL_IF_NOT(s->sp->port2 == 44);
+    FAIL_IF_NULL(s->sp->next);
+    FAIL_IF_NOT(s->sp->next->port == 46);
+    FAIL_IF_NOT(s->sp->next->port2 == 50);
+    FAIL_IF_NOT_NULL(s->sp->next->next);
+    DetectEngineCtxFree(de_ctx);
+    PASS;
+}
+
 /**
  * \brief this function registers unit tests for DetectParse
  */
@@ -63,4 +147,8 @@ void DetectParseRegisterTests(void)
 {
     UtRegisterTest("DetectParseTest01", DetectParseTest01);
     UtRegisterTest("DetectParseTestNoOpt", DetectParseTestNoOpt);
+    UtRegisterTest("SigParseTestNegatationNoWhitespace", SigParseTestNegatationNoWhitespace);
+    UtRegisterTest("SigParseTestWhitespaceLessThan14", SigParseTestWhitespaceLessThan14);
+    UtRegisterTest("SigParseTestWhitespace14Spaces", SigParseTestWhitespace14Spaces);
+    UtRegisterTest("SigParseTestWhitespaceMoreThan14", SigParseTestWhitespaceMoreThan14);
 }