aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

content: don't error out on incomplete hex

Browse Source 
Before 6.0.6 if hex content was incomplete, Suricata didn't error out.
With 6.0.6 incomplete hex was detected and errored on which is a
breaking change in a release branch.  Instead, only emit a warning
unless strict content checking has been requested.

To enable strict behaviour on incomplete content hex in a rule,
"--strict-rule-keywords=content" can be used on the command line.

Issue: #5546
pull/7887/head
Jason Ish 3 years ago
parent 726de4e70c
commit 8174ba9e6f
9 changed files with 86 additions and 60 deletions
Show Stats Download Patch File Download Diff File

68
src/detect-content.c
Unescape Escape View File

@ -23,6 +23,7 @@
* Simple content match part of the detection engine.
*/
#include "detect-engine-register.h"
#include "suricata-common.h"
#include "decode.h"
#include "detect.h"
@ -79,8 +80,8 @@ void DetectContentRegister (void)
* \retval -1 error
* \retval 0 ok
*/
int DetectContentDataParse(const char *keyword, const char *contentstr,
uint8_t **pstr, uint16_t *plen)
int DetectContentDataParse(
const char *keyword, const char *contentstr, uint8_t **pstr, uint16_t *plen, bool *warning)
{
char *str = NULL;
size_t slen = 0;
@ -112,9 +113,24 @@ int DetectContentDataParse(const char *keyword, const char *contentstr,
bin_count++;
if (bin) {
if (binpos > 0) {
SCLogError(SC_ERR_INVALID_SIGNATURE,
"Incomplete hex code in content - %s. Invalidating signature.",
contentstr);
if (SigMatchStrictEnabled(DETECT_CONTENT)) {
SCLogError(SC_ERR_INVALID_SIGNATURE,
"Incomplete hex code in content - %s. Invalidating signature.",
contentstr);
} else {
SCLogWarning(SC_ERR_INVALID_SIGNATURE,
"Incomplete hex code in content - %s. Invalidating signature. "
"This will become an error in Suricata 7.0.",
contentstr);
/* Prior to 6.0.6, incomplete hex was silently
ignored. With 6.0.6 this turned into an
error with -T. For 6.0.7, make the error
non-fatal unless strict content parsing is
enabled. */
if (warning != NULL) {
*warning = true;
}
}
goto error;
}
bin = 0;
@ -202,20 +218,21 @@ int DetectContentDataParse(const char *keyword, const char *contentstr,
error:
return -1;
}
/**
* \brief DetectContentParse
* \initonly
*/
DetectContentData *DetectContentParse(SpmGlobalThreadCtx *spm_global_thread_ctx,
const char *contentstr)
DetectContentData *DetectContentParse(
SpmGlobalThreadCtx *spm_global_thread_ctx, const char *contentstr, bool *warning)
{
DetectContentData *cd = NULL;
uint8_t *content = NULL;
uint16_t len = 0;
int ret;
ret = DetectContentDataParse("content", contentstr, &content, &len);
if (ret == -1) {
ret = DetectContentDataParse("content", contentstr, &content, &len, warning);
if (ret < 0) {
return NULL;
}
@ -253,7 +270,7 @@ DetectContentData *DetectContentParse(SpmGlobalThreadCtx *spm_global_thread_ctx,
DetectContentData *DetectContentParseEncloseQuotes(SpmGlobalThreadCtx *spm_global_thread_ctx,
const char *contentstr)
{
return DetectContentParse(spm_global_thread_ctx, contentstr);
return DetectContentParse(spm_global_thread_ctx, contentstr, NULL);
}
/**
@ -329,8 +346,9 @@ int DetectContentSetup(DetectEngineCtx *de_ctx, Signature *s, const char *conten
{
DetectContentData *cd = NULL;
SigMatch *sm = NULL;
bool warning = false;
cd = DetectContentParse(de_ctx->spm_global_thread_ctx, contentstr);
cd = DetectContentParse(de_ctx->spm_global_thread_ctx, contentstr, &warning);
if (cd == NULL)
goto error;
if (s->init_data->negated == true) {
@ -369,7 +387,7 @@ int DetectContentSetup(DetectEngineCtx *de_ctx, Signature *s, const char *conten
error:
DetectContentFree(de_ctx, cd);
return -1;
return warning ? -4 : -1;
}
/**
@ -796,7 +814,7 @@ static int DetectContentParseTest01 (void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd != NULL) {
if (memcmp(cd->content, teststringparsed, strlen(teststringparsed)) != 0) {
SCLogDebug("expected %s got ", teststringparsed);
@ -827,7 +845,7 @@ static int DetectContentParseTest02 (void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd != NULL) {
if (memcmp(cd->content, teststringparsed, strlen(teststringparsed)) != 0) {
SCLogDebug("expected %s got ", teststringparsed);
@ -858,7 +876,7 @@ static int DetectContentParseTest03 (void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd != NULL) {
if (memcmp(cd->content, teststringparsed, strlen(teststringparsed)) != 0) {
SCLogDebug("expected %s got ", teststringparsed);
@ -889,7 +907,7 @@ static int DetectContentParseTest04 (void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd != NULL) {
uint16_t len = (cd->content_len > strlen(teststringparsed));
if (memcmp(cd->content, teststringparsed, len) != 0) {
@ -920,7 +938,7 @@ static int DetectContentParseTest05 (void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd != NULL) {
SCLogDebug("expected NULL got ");
PrintRawUriFp(stdout,cd->content,cd->content_len);
@ -946,7 +964,7 @@ static int DetectContentParseTest06 (void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd != NULL) {
uint16_t len = (cd->content_len > strlen(teststringparsed));
if (memcmp(cd->content, teststringparsed, len) != 0) {
@ -977,7 +995,7 @@ static int DetectContentParseTest07 (void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd != NULL) {
SCLogDebug("expected NULL got %p: ", cd);
result = 0;
@ -1000,7 +1018,7 @@ static int DetectContentParseTest08 (void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd != NULL) {
SCLogDebug("expected NULL got %p: ", cd);
result = 0;
@ -1288,7 +1306,7 @@ static int DetectContentParseTest09(void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
FAIL_IF_NULL(cd);
DetectContentFree(NULL, cd);
SpmDestroyGlobalThreadCtx(spm_global_thread_ctx);
@ -2373,7 +2391,7 @@ static int DetectContentParseTest41(void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd == NULL) {
SCLogDebug("expected not NULL");
result = 0;
@ -2406,7 +2424,7 @@ static int DetectContentParseTest42(void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd == NULL) {
SCLogDebug("expected not NULL");
result = 0;
@ -2440,7 +2458,7 @@ static int DetectContentParseTest43(void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd == NULL) {
SCLogDebug("expected not NULL");
result = 0;
@ -2477,7 +2495,7 @@ static int DetectContentParseTest44(void)
SpmGlobalThreadCtx *spm_global_thread_ctx = SpmInitGlobalThreadCtx(spm_matcher);
FAIL_IF(spm_global_thread_ctx == NULL);
cd = DetectContentParse(spm_global_thread_ctx, teststring);
cd = DetectContentParse(spm_global_thread_ctx, teststring, NULL);
if (cd == NULL) {
SCLogDebug("expected not NULL");
result = 0;

8
src/detect-content.h
Unescape Escape View File

@ -109,10 +109,10 @@ typedef struct DetectContentData_ {
/* prototypes */
void DetectContentRegister (void);
uint32_t DetectContentMaxId(DetectEngineCtx *);
DetectContentData *DetectContentParse(SpmGlobalThreadCtx *spm_global_thread_ctx,
const char *contentstr);
int DetectContentDataParse(const char *keyword, const char *contentstr,
uint8_t **pstr, uint16_t *plen);
DetectContentData *DetectContentParse(
SpmGlobalThreadCtx *spm_global_thread_ctx, const char *contentstr, bool *warning);
int DetectContentDataParse(
const char *keyword, const char *contentstr, uint8_t **pstr, uint16_t *plen, bool *warning);
DetectContentData *DetectContentParseEncloseQuotes(SpmGlobalThreadCtx *spm_global_thread_ctx,
const char *contentstr);

17
src/detect-fileext.c
Unescape Escape View File

@ -138,7 +138,8 @@ static int DetectFileextMatch (DetectEngineThreadCtx *det_ctx,
* \retval pointer to DetectFileextData on success
* \retval NULL on failure
*/
static DetectFileextData *DetectFileextParse (DetectEngineCtx *de_ctx, const char *str, bool negate)
static DetectFileextData *DetectFileextParse(
DetectEngineCtx *de_ctx, const char *str, bool negate, bool *warning)
{
DetectFileextData *fileext = NULL;
@ -149,7 +150,7 @@ static DetectFileextData *DetectFileextParse (DetectEngineCtx *de_ctx, const cha
memset(fileext, 0x00, sizeof(DetectFileextData));
if (DetectContentDataParse("fileext", str, &fileext->ext, &fileext->len) == -1) {
if (DetectContentDataParse("fileext", str, &fileext->ext, &fileext->len, warning) == -1) {
goto error;
}
uint16_t u;
@ -201,8 +202,9 @@ static int DetectFileextSetup (DetectEngineCtx *de_ctx, Signature *s, const char
{
DetectFileextData *fileext= NULL;
SigMatch *sm = NULL;
bool warning = false;
fileext = DetectFileextParse(de_ctx, str, s->init_data->negated);
fileext = DetectFileextParse(de_ctx, str, s->init_data->negated, &warning);
if (fileext == NULL)
goto error;
@ -225,8 +227,7 @@ error:
DetectFileextFree(de_ctx, fileext);
if (sm != NULL)
SCFree(sm);
return -1;
return warning ? -4 : -1;
}
/**
@ -251,7 +252,7 @@ static void DetectFileextFree(DetectEngineCtx *de_ctx, void *ptr)
*/
static int DetectFileextTestParse01 (void)
{
DetectFileextData *dfd = DetectFileextParse(NULL, "doc", false);
DetectFileextData *dfd = DetectFileextParse(NULL, "doc", false, NULL);
if (dfd != NULL) {
DetectFileextFree(NULL, dfd);
return 1;
@ -266,7 +267,7 @@ static int DetectFileextTestParse02 (void)
{
int result = 0;
DetectFileextData *dfd = DetectFileextParse(NULL, "tar.gz", false);
DetectFileextData *dfd = DetectFileextParse(NULL, "tar.gz", false, NULL);
if (dfd != NULL) {
if (dfd->len == 6 && memcmp(dfd->ext, "tar.gz", 6) == 0) {
result = 1;
@ -285,7 +286,7 @@ static int DetectFileextTestParse03 (void)
{
int result = 0;
DetectFileextData *dfd = DetectFileextParse(NULL, "pdf", false);
DetectFileextData *dfd = DetectFileextParse(NULL, "pdf", false, NULL);
if (dfd != NULL) {
if (dfd->len == 3 && memcmp(dfd->ext, "pdf", 3) == 0) {
result = 1;

18
src/detect-filemagic.c
Unescape Escape View File

@ -262,7 +262,8 @@ static int DetectFilemagicMatch (DetectEngineThreadCtx *det_ctx,
* \retval filemagic pointer to DetectFilemagicData on success
* \retval NULL on failure
*/
static DetectFilemagicData *DetectFilemagicParse (DetectEngineCtx *de_ctx, const char *str, bool negate)
static DetectFilemagicData *DetectFilemagicParse(
DetectEngineCtx *de_ctx, const char *str, bool negate, bool *warning)
{
DetectFilemagicData *filemagic = NULL;
@ -273,7 +274,8 @@ static DetectFilemagicData *DetectFilemagicParse (DetectEngineCtx *de_ctx, const
memset(filemagic, 0x00, sizeof(DetectFilemagicData));
if (DetectContentDataParse ("filemagic", str, &filemagic->name, &filemagic->len) == -1) {
if (DetectContentDataParse("filemagic", str, &filemagic->name, &filemagic->len, warning) ==
-1) {
goto error;
}
@ -356,10 +358,12 @@ static void DetectFilemagicThreadFree(void *ctx)
static int DetectFilemagicSetup (DetectEngineCtx *de_ctx, Signature *s, const char *str)
{
SigMatch *sm = NULL;
bool warning = false;
DetectFilemagicData *filemagic = DetectFilemagicParse(de_ctx, str, s->init_data->negated);
DetectFilemagicData *filemagic =
DetectFilemagicParse(de_ctx, str, s->init_data->negated, &warning);
if (filemagic == NULL)
return -1;
return warning ? -4 : -1;
g_magic_thread_ctx_id = DetectRegisterThreadCtxFuncs(de_ctx, "filemagic",
DetectFilemagicThreadInit, (void *)filemagic, DetectFilemagicThreadFree, 1);
@ -585,7 +589,7 @@ static int PrefilterMpmFilemagicRegister(DetectEngineCtx *de_ctx,
*/
static int DetectFilemagicTestParse01 (void)
{
DetectFilemagicData *dnd = DetectFilemagicParse(NULL, "secret.pdf", false);
DetectFilemagicData *dnd = DetectFilemagicParse(NULL, "secret.pdf", false, NULL);
if (dnd != NULL) {
DetectFilemagicFree(NULL, dnd);
return 1;
@ -600,7 +604,7 @@ static int DetectFilemagicTestParse02 (void)
{
int result = 0;
DetectFilemagicData *dnd = DetectFilemagicParse(NULL, "backup.tar.gz", false);
DetectFilemagicData *dnd = DetectFilemagicParse(NULL, "backup.tar.gz", false, NULL);
if (dnd != NULL) {
if (dnd->len == 13 && memcmp(dnd->name, "backup.tar.gz", 13) == 0) {
result = 1;
@ -619,7 +623,7 @@ static int DetectFilemagicTestParse03 (void)
{
int result = 0;
DetectFilemagicData *dnd = DetectFilemagicParse(NULL, "cmd.exe", false);
DetectFilemagicData *dnd = DetectFilemagicParse(NULL, "cmd.exe", false, NULL);
if (dnd != NULL) {
if (dnd->len == 7 && memcmp(dnd->name, "cmd.exe", 7) == 0) {
result = 1;

16
src/detect-filename.c
Unescape Escape View File

@ -233,7 +233,8 @@ static int DetectFilenameMatch (DetectEngineThreadCtx *det_ctx,
* \retval filename pointer to DetectFilenameData on success
* \retval NULL on failure
*/
static DetectFilenameData *DetectFilenameParse (DetectEngineCtx *de_ctx, const char *str, bool negate)
static DetectFilenameData *DetectFilenameParse(
DetectEngineCtx *de_ctx, const char *str, bool negate, bool *warning)
{
DetectFilenameData *filename = NULL;
@ -244,7 +245,7 @@ static DetectFilenameData *DetectFilenameParse (DetectEngineCtx *de_ctx, const c
memset(filename, 0x00, sizeof(DetectFilenameData));
if (DetectContentDataParse ("filename", str, &filename->name, &filename->len) == -1) {
if (DetectContentDataParse("filename", str, &filename->name, &filename->len, warning) == -1) {
goto error;
}
@ -297,8 +298,9 @@ static int DetectFilenameSetup (DetectEngineCtx *de_ctx, Signature *s, const cha
{
DetectFilenameData *filename = NULL;
SigMatch *sm = NULL;
bool warning = false;
filename = DetectFilenameParse(de_ctx, str, s->init_data->negated);
filename = DetectFilenameParse(de_ctx, str, s->init_data->negated, &warning);
if (filename == NULL)
goto error;
@ -321,7 +323,7 @@ error:
DetectFilenameFree(de_ctx, filename);
if (sm != NULL)
SCFree(sm);
return -1;
return warning ? -4 : -1;
}
/**
@ -520,7 +522,7 @@ static int DetectFilenameSignatureParseTest01(void)
*/
static int DetectFilenameTestParse01 (void)
{
DetectFilenameData *dnd = DetectFilenameParse(NULL, "secret.pdf", false);
DetectFilenameData *dnd = DetectFilenameParse(NULL, "secret.pdf", false, NULL);
if (dnd != NULL) {
DetectFilenameFree(NULL, dnd);
return 1;
@ -535,7 +537,7 @@ static int DetectFilenameTestParse02 (void)
{
int result = 0;
DetectFilenameData *dnd = DetectFilenameParse(NULL, "backup.tar.gz", false);
DetectFilenameData *dnd = DetectFilenameParse(NULL, "backup.tar.gz", false, NULL);
if (dnd != NULL) {
if (dnd->len == 13 && memcmp(dnd->name, "backup.tar.gz", 13) == 0) {
result = 1;
@ -554,7 +556,7 @@ static int DetectFilenameTestParse03 (void)
{
int result = 0;
DetectFilenameData *dnd = DetectFilenameParse(NULL, "cmd.exe", false);
DetectFilenameData *dnd = DetectFilenameParse(NULL, "cmd.exe", false, NULL);
if (dnd != NULL) {
if (dnd->len == 7 && memcmp(dnd->name, "cmd.exe", 7) == 0) {
result = 1;

6
src/detect-flowvar.c
Unescape Escape View File

@ -122,6 +122,7 @@ static int DetectFlowvarSetup (DetectEngineCtx *de_ctx, Signature *s, const char
uint8_t *content = NULL;
uint16_t contentlen = 0;
uint32_t contentflags = s->init_data->negated ? DETECT_CONTENT_NEGATED : 0;
bool warning = false;
ret = DetectParsePcreExec(&parse_regex, rawstr, 0, 0, ov, MAX_SUBSTRINGS);
if (ret != 3) {
@ -150,7 +151,8 @@ static int DetectFlowvarSetup (DetectEngineCtx *de_ctx, Signature *s, const char
}
SCLogDebug("varcontent %s", &varcontent[varcontent_index]);
res = DetectContentDataParse("flowvar", &varcontent[varcontent_index], &content, &contentlen);
res = DetectContentDataParse(
"flowvar", &varcontent[varcontent_index], &content, &contentlen, &warning);
if (res == -1)
goto error;
@ -193,7 +195,7 @@ error:
SCFree(sm);
if (content != NULL)
SCFree(content);
return -1;
return warning ? -4 : -1;
}
/** \brief Store flowvar in det_ctx so we can exec it post-match */

6
src/detect-parse.c
Unescape Escape View File

@ -1911,13 +1911,11 @@ static Signature *SigInitHelper(DetectEngineCtx *de_ctx, const char *sigstr,
if (ret == -4) {
de_ctx->sigerror_ok = true;
goto error;
}
else if (ret == -3) {
} else if (ret == -3) {
de_ctx->sigerror_silent = true;
de_ctx->sigerror_ok = true;
goto error;
}
else if (ret == -2) {
} else if (ret == -2) {
de_ctx->sigerror_silent = true;
goto error;
} else if (ret < 0) {

2
src/detect-pktvar.c
Unescape Escape View File

@ -127,7 +127,7 @@ static int DetectPktvarSetup (DetectEngineCtx *de_ctx, Signature *s, const char
parse_content = varcontent;
}
ret = DetectContentDataParse("pktvar", parse_content, &content, &len);
ret = DetectContentDataParse("pktvar", parse_content, &content, &len, NULL);
if (ret == -1 || content == NULL) {
pcre_free(varname);
pcre_free(varcontent);

5
src/detect-replace.c
Unescape Escape View File

@ -93,6 +93,7 @@ int DetectReplaceSetup(DetectEngineCtx *de_ctx, Signature *s, const char *replac
{
uint8_t *content = NULL;
uint16_t len = 0;
bool warning = false;
if (s->init_data->negated) {
SCLogError(SC_ERR_INVALID_VALUE, "Can't negate replacement string: %s",
@ -112,9 +113,9 @@ int DetectReplaceSetup(DetectEngineCtx *de_ctx, Signature *s, const char *replac
return 0;
}
int ret = DetectContentDataParse("replace", replacestr, &content, &len);
int ret = DetectContentDataParse("replace", replacestr, &content, &len, &warning);
if (ret == -1)
return -1;
return warning ? -4 : -1;
/* add to the latest "content" keyword from pmatch */
const SigMatch *pm = DetectGetLastSMByListId(s, DETECT_SM_LIST_PMATCH,

Write Preview
Loading…
Cancel
Save