Skip to content

Commit 9df5fd1

Browse files
inashivbvictorjulien
authored andcommitted
flowbits: add a validation callback during setup
This should make it possible to catch invalid combinations in the same signature early. This patch covers checking and erroring on the following invalid cmd combinations: - set + isset - unset + isnotset - set + toggle - set + unset - isset + isnotset - unset + toggle the same flowbit in the same signature which is basically an unnecessary operation at runtime. This also helps bring down the difficulty of handling of actual complex flowbit chains. Bug 7772 Bug 7773 Bug 7774 Bug 7817 Bug 7818 Bug 8166
1 parent b575ae3 commit 9df5fd1

1 file changed

Lines changed: 98 additions & 0 deletions

File tree

src/detect-flowbits.c

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747

4848
#include "tree.h"
4949

50+
#include "util-enum.h"
5051
#include "util-var-name.h"
5152
#include "util-unittest.h"
5253
#include "util-debug.h"
@@ -91,6 +92,98 @@ void DetectFlowbitsRegister (void)
9192
DetectSetupParseRegexes(PARSE_REGEX, &parse_regex);
9293
}
9394

95+
static bool DetectFlowbitIsPostmatch(uint8_t cmd)
96+
{
97+
DEBUG_VALIDATE_BUG_ON(cmd >= DETECT_FLOWBITS_CMD_MAX);
98+
99+
switch (cmd) {
100+
case DETECT_FLOWBITS_CMD_TOGGLE:
101+
case DETECT_FLOWBITS_CMD_SET:
102+
case DETECT_FLOWBITS_CMD_UNSET:
103+
return true;
104+
}
105+
return false;
106+
}
107+
108+
SCEnumCharMap flowbit_cmds[] = {
109+
{ "set", DETECT_FLOWBITS_CMD_SET },
110+
{ "toggle", DETECT_FLOWBITS_CMD_TOGGLE },
111+
{ "unset", DETECT_FLOWBITS_CMD_UNSET },
112+
{ "isnotset", DETECT_FLOWBITS_CMD_ISNOTSET },
113+
{ "isset", DETECT_FLOWBITS_CMD_ISSET },
114+
};
115+
116+
static inline int DetectFlowbitValidateDo(
117+
Signature *s, uint8_t cmd, uint8_t cmd2, uint32_t idx, bool err)
118+
{
119+
bool postmatch = DetectFlowbitIsPostmatch(cmd);
120+
SigMatch *list = postmatch ? s->init_data->smlists[DETECT_SM_LIST_POSTMATCH]
121+
: s->init_data->smlists[DETECT_SM_LIST_MATCH];
122+
123+
for (SigMatch *sm = list; sm != NULL; sm = sm->next) {
124+
if (sm->type != DETECT_FLOWBITS)
125+
continue;
126+
127+
DetectFlowbitsData *fd = (DetectFlowbitsData *)sm->ctx;
128+
if ((fd->idx == idx) && (fd->cmd == cmd)) {
129+
if (err) {
130+
SCLogError("invalid flowbit command combination in the same signature: isset and "
131+
"isnotset");
132+
return -1;
133+
}
134+
SCLogWarning(
135+
"inconsequential flowbit command combination in the same signature: %s and %s",
136+
SCMapEnumValueToName(cmd, flowbit_cmds),
137+
SCMapEnumValueToName(cmd2, flowbit_cmds));
138+
return 0;
139+
}
140+
}
141+
142+
/* no invalid or inconsequential command pair was found */
143+
return 1;
144+
}
145+
146+
static int DetectFlowbitValidate(Signature *s, DetectFlowbitsData *fd)
147+
{
148+
struct DetectFlowbitInvalidCmdMap_ {
149+
uint8_t cmd1;
150+
uint8_t cmd2;
151+
bool err; /* Error out if rule is unsatisfiable at runtime */
152+
};
153+
154+
struct DetectFlowbitInvalidCmdMap_ icmds_map[] = {
155+
/* POSTMATCH, MATCH combinations */
156+
{ DETECT_FLOWBITS_CMD_UNSET, DETECT_FLOWBITS_CMD_ISNOTSET, false },
157+
{ DETECT_FLOWBITS_CMD_SET, DETECT_FLOWBITS_CMD_ISSET, false },
158+
/* POSTMATCH, POSTMATCH combinations */
159+
{ DETECT_FLOWBITS_CMD_SET, DETECT_FLOWBITS_CMD_TOGGLE, false },
160+
{ DETECT_FLOWBITS_CMD_UNSET, DETECT_FLOWBITS_CMD_TOGGLE, false },
161+
{ DETECT_FLOWBITS_CMD_SET, DETECT_FLOWBITS_CMD_UNSET, false },
162+
/* MATCH, MATCH combinations */
163+
{ DETECT_FLOWBITS_CMD_ISSET, DETECT_FLOWBITS_CMD_ISNOTSET, true },
164+
};
165+
166+
int ret = 0;
167+
168+
for (uint8_t i = 0; i < ARRAY_SIZE(icmds_map); i++) {
169+
if (fd->cmd == icmds_map[i].cmd1) {
170+
ret = DetectFlowbitValidateDo(
171+
s, icmds_map[i].cmd2, icmds_map[i].cmd1, fd->idx, icmds_map[i].err);
172+
if (ret != 1) {
173+
return ret;
174+
}
175+
} else if (fd->cmd == icmds_map[i].cmd2) {
176+
ret = DetectFlowbitValidateDo(
177+
s, icmds_map[i].cmd1, icmds_map[i].cmd2, fd->idx, icmds_map[i].err);
178+
if (ret != 1) {
179+
return ret;
180+
}
181+
}
182+
}
183+
184+
return 0;
185+
}
186+
94187
static int FlowbitOrAddData(DetectEngineCtx *de_ctx, DetectFlowbitsData *cd, char *arrptr)
95188
{
96189
char *strarr[MAX_TOKENS];
@@ -363,6 +456,11 @@ int DetectFlowbitSetup (DetectEngineCtx *de_ctx, Signature *s, const char *rawst
363456
SCLogDebug("idx %" PRIu32 ", cmd %s, name %s",
364457
cd->idx, fb_cmd_str, strlen(fb_name) ? fb_name : "(none)");
365458
}
459+
460+
if (DetectFlowbitValidate(s, cd) != 0) {
461+
goto error;
462+
}
463+
366464
/* Okay so far so good, lets get this into a SigMatch
367465
* and put it in the Signature. */
368466

0 commit comments

Comments
 (0)