Skip to content

Commit 10ea60a

Browse files
giannitedescovictorjulien
authored andcommitted
detect: Validate that NOOPT options don't have optvals
Without this, a simple typo between : and ; is able to hide actual bugs in rules. I discovered 2 bugs in ET open ruleset this way.
1 parent cebe15c commit 10ea60a

File tree

2 files changed

+26
-2
lines changed

2 files changed

+26
-2
lines changed

src/detect-parse.c

+8-2
Original file line numberDiff line numberDiff line change
@@ -704,8 +704,14 @@ static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr,
704704

705705
if (!(st->flags & (SIGMATCH_NOOPT|SIGMATCH_OPTIONAL_OPT))) {
706706
if (optvalue == NULL || strlen(optvalue) == 0) {
707-
SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid formatting or malformed option to %s keyword: \'%s\'",
708-
optname, optstr);
707+
SCLogError(SC_ERR_INVALID_SIGNATURE,
708+
"invalid formatting or malformed option to %s keyword: '%s'", optname, optstr);
709+
goto error;
710+
}
711+
} else if (st->flags & SIGMATCH_NOOPT) {
712+
if (optvalue && strlen(optvalue)) {
713+
SCLogError(SC_ERR_INVALID_SIGNATURE, "unexpected option to %s keyword: '%s'", optname,
714+
optstr);
709715
goto error;
710716
}
711717
}

src/tests/detect-parse.c

+18
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,29 @@ static int DetectParseTest01 (void)
3838
PASS;
3939
}
4040

41+
/**
42+
* \test DetectParseTestNoOpt is a regression test to make sure that we reject
43+
* any signature where a NOOPT rule option is given a value. This can hide rule
44+
* errors which make other options disappear, eg: foo: bar: baz; where "foo" is
45+
* the NOOPT option, we will end up with a signature which is missing "bar".
46+
*/
47+
48+
static int DetectParseTestNoOpt(void)
49+
{
50+
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
51+
FAIL_IF(DetectEngineAppendSig(de_ctx,
52+
"alert http any any -> any any (msg:\"sid 1 version 0\"; "
53+
"content:\"dummy1\"; endswith: reference: ref; sid:1;)") != NULL);
54+
DetectEngineCtxFree(de_ctx);
55+
56+
PASS;
57+
}
4158

4259
/**
4360
* \brief this function registers unit tests for DetectParse
4461
*/
4562
void DetectParseRegisterTests(void)
4663
{
4764
UtRegisterTest("DetectParseTest01", DetectParseTest01);
65+
UtRegisterTest("DetectParseTestNoOpt", DetectParseTestNoOpt);
4866
}

0 commit comments

Comments
 (0)