Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Detect prefilter setupu8 4335 v6 #5937

Closed
wants to merge 6 commits into from
Closed

Detect prefilter setupu8 4335 v6 #5937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8b779dc
detect: errors for rule with impossible conditions
catenacyber Feb 17, 2021
10f69db
detect: use prefilter values for modes
catenacyber Feb 17, 2021
73c6b5e
detect: allows <> syntax for uint ranges
catenacyber Feb 18, 2021
d14b43a
detect: corrects reverted ranges and issues a warning
catenacyber Feb 18, 2021
f3e7a3e
detect: use generic functions for icode parsing
catenacyber Feb 17, 2021
b2e01c6
detect: adds test with invalid uint mode <<
catenacyber Feb 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
155 changes: 120 additions & 35 deletions src/detect-engine-uint.c
View file
Open in desktop
Expand Up @@ -65,6 +65,33 @@ int DetectU32Match(const uint32_t parg, const DetectU32Data *du32)
return 0;
}

static int DetectU32Validate(DetectU32Data *du32)
{
switch (du32->mode) {
case DETECT_UINT_LT:
if (du32->arg1 == 0) {
return 1;
}
break;
case DETECT_UINT_GT:
if (du32->arg1 == UINT32_MAX) {
return 1;
}
break;
case DETECT_UINT_RA:
if (du32->arg1 >= du32->arg2) {
return 1;
}
// we need at least one value that can match parg > du32->arg1 && parg < du32->arg2
if (du32->arg1 + 1 >= du32->arg2) {
return 1;
}
break;
default:
break;
}
return 0;
}

/**
* \brief This function is used to parse u32 options passed via some u32 keyword
Expand Down Expand Up @@ -124,24 +151,33 @@ DetectU32Data *DetectU32Parse (const char *u32str)
switch(arg2[0]) {
case '<':
case '>':
if (strlen(arg3) == 0)
return NULL;

if (ByteExtractStringUint32(&u32da.arg1, 10, strlen(arg3), arg3) < 0) {
SCLogError(SC_ERR_BYTE_EXTRACT_FAILED, "ByteExtractStringUint32 failed");
return NULL;
}

SCLogDebug("u32 is %"PRIu32"",u32da.arg1);
if (strlen(arg1) > 0)
if (strlen(arg2) == 1) {
if (strlen(arg3) == 0)
return NULL;

if (ByteExtractStringUint32(&u32da.arg1, 10, strlen(arg3), arg3) < 0) {
SCLogError(SC_ERR_BYTE_EXTRACT_FAILED, "ByteExtractStringUint32 failed");
return NULL;
}

SCLogDebug("u32 is %" PRIu32 "", u32da.arg1);
if (strlen(arg1) > 0)
return NULL;

if (arg2[0] == '<') {
u32da.mode = DETECT_UINT_LT;
} else { // arg2[0] == '>'
u32da.mode = DETECT_UINT_GT;
}
break;
} else if (strlen(arg2) == 2) {
if (arg2[0] != '<' || arg2[1] != '>') {
return NULL;
}
} else {
return NULL;

if (arg2[0] == '<') {
u32da.mode = DETECT_UINT_LT;
} else { // arg2[0] == '>'
u32da.mode = DETECT_UINT_GT;
}
break;
// fall through
case '-':
if (strlen(arg1)== 0)
return NULL;
Expand All @@ -159,9 +195,11 @@ DetectU32Data *DetectU32Parse (const char *u32str)
}

SCLogDebug("u32 is %"PRIu32" to %"PRIu32"", u32da.arg1, u32da.arg2);
if (u32da.arg1 >= u32da.arg2) {
SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid u32 range. ");
return NULL;
if (u32da.arg1 > u32da.arg2) {
uint32_t temp = u32da.arg1;
u32da.arg1 = u32da.arg2;
u32da.arg2 = temp;
SCLogWarning(SC_WARN_POOR_RULE, "Reversed u32 range. ");
}
break;
default:
Expand Down Expand Up @@ -189,6 +227,10 @@ DetectU32Data *DetectU32Parse (const char *u32str)
return NULL;
}
}
if (DetectU32Validate(&u32da)) {
SCLogError(SC_ERR_INVALID_VALUE, "Impossible value for uint32 condition");
return NULL;
}
u32d = SCCalloc(1, sizeof (DetectU32Data));
if (unlikely(u32d == NULL))
return NULL;
Expand Down Expand Up @@ -260,6 +302,34 @@ int DetectU8Match(const uint8_t parg, const DetectU8Data *du8)
return 0;
}

static int DetectU8Validate(DetectU8Data *du8)
{
switch (du8->mode) {
case DETECT_UINT_LT:
if (du8->arg1 == 0) {
return 1;
}
break;
case DETECT_UINT_GT:
if (du8->arg1 == UINT8_MAX) {
return 1;
}
break;
case DETECT_UINT_RA:
if (du8->arg1 >= du8->arg2) {
return 1;
}
// we need at least one value that can match parg > du8->arg1 && parg < du8->arg2
if (du8->arg1 + 1 >= du8->arg2) {
return 1;
}
break;
default:
break;
}
return 0;
}

/**
* \brief This function is used to parse u8 options passed via some u8 keyword
*
Expand Down Expand Up @@ -318,21 +388,30 @@ DetectU8Data *DetectU8Parse (const char *u8str)
switch(arg2[0]) {
case '<':
case '>':
if (StringParseUint8(&u8da.arg1, 10, strlen(arg3), arg3) < 0) {
SCLogError(SC_ERR_BYTE_EXTRACT_FAILED, "ByteExtractStringUint8 failed");
if (strlen(arg2) == 1) {
if (StringParseUint8(&u8da.arg1, 10, strlen(arg3), arg3) < 0) {
SCLogError(SC_ERR_BYTE_EXTRACT_FAILED, "ByteExtractStringUint8 failed");
return NULL;
}

SCLogDebug("u8 is %" PRIu8 "", u8da.arg1);
if (strlen(arg1) > 0)
return NULL;

if (arg2[0] == '<') {
u8da.mode = DETECT_UINT_LT;
} else { // arg2[0] == '>'
u8da.mode = DETECT_UINT_GT;
}
break;
} else if (strlen(arg2) == 2) {
if (arg2[0] != '<' || arg2[1] != '>') {
return NULL;
}
} else {
return NULL;
}

SCLogDebug("u8 is %"PRIu8"",u8da.arg1);
if (strlen(arg1) > 0)
return NULL;

if (arg2[0] == '<') {
u8da.mode = DETECT_UINT_LT;
} else { // arg2[0] == '>'
u8da.mode = DETECT_UINT_GT;
}
break;
// fallthrough
case '-':
u8da.mode = DETECT_UINT_RA;
if (StringParseUint8(&u8da.arg1, 10, strlen(arg1), arg1) < 0) {
Expand All @@ -345,9 +424,11 @@ DetectU8Data *DetectU8Parse (const char *u8str)
}

SCLogDebug("u8 is %"PRIu8" to %"PRIu8"", u8da.arg1, u8da.arg2);
if (u8da.arg1 >= u8da.arg2) {
SCLogError(SC_ERR_INVALID_SIGNATURE, "Invalid u8 range. ");
return NULL;
if (u8da.arg1 > u8da.arg2) {
uint8_t temp = u8da.arg1;
u8da.arg1 = u8da.arg2;
u8da.arg2 = temp;
SCLogWarning(SC_WARN_POOR_RULE, "Reversed u8 range. ");
}
break;
default:
Expand All @@ -373,6 +454,10 @@ DetectU8Data *DetectU8Parse (const char *u8str)
return NULL;
}
}
if (DetectU8Validate(&u8da)) {
SCLogError(SC_ERR_INVALID_VALUE, "Impossible value for uint8 condition");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we provide a more helpful error here? Something that includes the keyword name and perhaps also the original value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for the original value.
For the keyword name, should it be the caller of DetectU32Parse (such as DetectHTTP2windowSetup) which should print another message or should it transmit the keyword name to DetectU32Parse just for this debugging purpose ?

return NULL;
}
u8d = SCCalloc(1, sizeof (DetectU8Data));
if (unlikely(u8d == NULL))
return NULL;
Expand Down
8 changes: 4 additions & 4 deletions src/detect-engine-uint.h
View file
Open in desktop
Expand Up @@ -27,10 +27,10 @@
#include "detect-engine-prefilter-common.h"

typedef enum {
DETECT_UINT_LT,
DETECT_UINT_EQ,
DETECT_UINT_GT,
DETECT_UINT_RA,
DETECT_UINT_LT = PREFILTER_U8HASH_MODE_LT,
DETECT_UINT_EQ = PREFILTER_U8HASH_MODE_EQ,
DETECT_UINT_GT = PREFILTER_U8HASH_MODE_GT,
DETECT_UINT_RA = PREFILTER_U8HASH_MODE_RA,
} DetectUintMode;

typedef struct DetectU32Data_ {
Expand Down