Skip to content

Commit

Permalink
detect: SigMatchAppendSMToList can fail
Browse files Browse the repository at this point in the history 
Ticket: OISF#6104

And failures should be handled to say that the rule failed to load

Reverts the fix by 299ee6e
that was simple, but not complete (memory leak),
to have this bigger API change which simplifies code.
  • Loading branch information
@catenacyber @victorjulien
catenacyber authored and victorjulien committed Nov 21, 2023
1 parent e38b9de commit c272a64
Show file tree
Hide file tree
Showing 106 changed files with 495 additions and 1,169 deletions.
21 changes: 8 additions & 13 deletions src/detect-app-layer-event.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,20 +278,19 @@ static int DetectAppLayerEventSetup(DetectEngineCtx *de_ctx, Signature *s, const
}
SCLogDebug("data->event_id %u", data->event_id);

SigMatch *sm = SigMatchAlloc();
if (sm == NULL)
goto error;

sm->type = DETECT_AL_APP_LAYER_EVENT;
sm->ctx = (SigMatchCtx *)data;

if (event_type == APP_LAYER_EVENT_TYPE_PACKET) {
SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_MATCH);
if (SigMatchAppendSMToList(de_ctx, s, DETECT_AL_APP_LAYER_EVENT, (SigMatchCtx *)data,
DETECT_SM_LIST_MATCH) == NULL) {
goto error;
}
} else {
if (DetectSignatureSetAppProto(s, data->alproto) != 0)
goto error;

SigMatchAppendSMToList(s, sm, g_applayer_events_list_id);
if (SigMatchAppendSMToList(de_ctx, s, DETECT_AL_APP_LAYER_EVENT, (SigMatchCtx *)data,
g_applayer_events_list_id) == NULL) {
goto error;
}
s->flags |= SIG_FLAG_APPLAYER;
}

Expand All @@ -301,10 +300,6 @@ static int DetectAppLayerEventSetup(DetectEngineCtx *de_ctx, Signature *s, const
if (data) {
DetectAppLayerEventFree(de_ctx, data);
}
if (sm) {
sm->ctx = NULL;
SigMatchFree(de_ctx, sm);
}
return -1;
}

Expand Down
11 changes: 3 additions & 8 deletions src/detect-app-layer-protocol.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ static int DetectAppLayerProtocolSetup(DetectEngineCtx *de_ctx,
Signature *s, const char *arg)
{
DetectAppLayerProtocolData *data = NULL;
SigMatch *sm = NULL;

if (s->alproto != ALPROTO_UNKNOWN) {
SCLogError("Either we already "
Expand Down Expand Up @@ -169,14 +168,10 @@ static int DetectAppLayerProtocolSetup(DetectEngineCtx *de_ctx,
}
}

sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_AL_APP_LAYER_PROTOCOL, (SigMatchCtx *)data,
DETECT_SM_LIST_MATCH) == NULL) {
goto error;

sm->type = DETECT_AL_APP_LAYER_PROTOCOL;
sm->ctx = (void *)data;

SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_MATCH);
}
return 0;

error:
Expand Down
11 changes: 2 additions & 9 deletions src/detect-asn1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,19 +127,12 @@ static int DetectAsn1Setup(DetectEngineCtx *de_ctx, Signature *s, const char *as
if (ad == NULL)
return -1;

/* Okay so far so good, lets get this into a SigMatch
* and put it in the Signature. */
SigMatch *sm = SigMatchAlloc();
if (sm == NULL) {
if (SigMatchAppendSMToList(de_ctx, s, DETECT_ASN1, (SigMatchCtx *)ad, DETECT_SM_LIST_MATCH) ==
NULL) {
DetectAsn1Free(de_ctx, ad);
return -1;
}

sm->type = DETECT_ASN1;
sm->ctx = (SigMatchCtx *)ad;

SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_MATCH);

return 0;
}

Expand Down
8 changes: 2 additions & 6 deletions src/detect-base64-decode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ static int DetectBase64DecodeSetup(DetectEngineCtx *de_ctx, Signature *s,
uint8_t relative = 0;
DetectBase64Decode *data = NULL;
int sm_list;
SigMatch *sm = NULL;
SigMatch *pm = NULL;

if (str != NULL) {
Expand Down Expand Up @@ -226,13 +225,10 @@ static int DetectBase64DecodeSetup(DetectEngineCtx *de_ctx, Signature *s,
}
}

sm = SigMatchAlloc();
if (sm == NULL) {
if (SigMatchAppendSMToList(de_ctx, s, DETECT_BASE64_DECODE, (SigMatchCtx *)data, sm_list) ==
NULL) {
goto error;
}
sm->type = DETECT_BASE64_DECODE;
sm->ctx = (SigMatchCtx *)data;
SigMatchAppendSMToList(s, sm, sm_list);

if (!data->bytes) {
data->bytes = BASE64_DECODE_MAX;
Expand Down
9 changes: 2 additions & 7 deletions src/detect-bsize.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ static int SigParseGetMaxBsize(DetectU64Data *bsz)
static int DetectBsizeSetup (DetectEngineCtx *de_ctx, Signature *s, const char *sizestr)
{
SCEnter();
SigMatch *sm = NULL;

if (DetectBufferGetActiveList(de_ctx, s) == -1)
SCReturnInt(-1);
Expand All @@ -212,13 +211,9 @@ static int DetectBsizeSetup (DetectEngineCtx *de_ctx, Signature *s, const char *
if (bsz == NULL)
goto error;

sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_BSIZE, (SigMatchCtx *)bsz, list) == NULL) {
goto error;
sm->type = DETECT_BSIZE;
sm->ctx = (void *)bsz;

SigMatchAppendSMToList(s, sm, list);
}

SCReturnInt(0);

Expand Down
9 changes: 2 additions & 7 deletions src/detect-bypass.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,16 @@ void DetectBypassRegister(void)

static int DetectBypassSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str)
{
SigMatch *sm = NULL;

if (s->flags & SIG_FLAG_FILESTORE) {
SCLogError("bypass can't work with filestore keyword");
return -1;
}
s->flags |= SIG_FLAG_BYPASS;

sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_BYPASS, NULL, DETECT_SM_LIST_POSTMATCH) == NULL) {
return -1;

sm->type = DETECT_BYPASS;
sm->ctx = NULL;
SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_POSTMATCH);
}

return 0;
}
Expand Down
11 changes: 3 additions & 8 deletions src/detect-byte-extract.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,6 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
*/
static int DetectByteExtractSetup(DetectEngineCtx *de_ctx, Signature *s, const char *arg)
{
SigMatch *sm = NULL;
SigMatch *prev_pm = NULL;
DetectByteExtractData *data = NULL;
int ret = -1;
Expand Down Expand Up @@ -609,14 +608,10 @@ static int DetectByteExtractSetup(DetectEngineCtx *de_ctx, Signature *s, const c
if (data->local_id > de_ctx->byte_extract_max_local_id)
de_ctx->byte_extract_max_local_id = data->local_id;


sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_BYTE_EXTRACT, (SigMatchCtx *)data, sm_list) ==
NULL) {
goto error;
sm->type = DETECT_BYTE_EXTRACT;
sm->ctx = (void *)data;
SigMatchAppendSMToList(s, sm, sm_list);

}

if (!(data->flags & DETECT_BYTE_EXTRACT_FLAG_RELATIVE))
goto okay;
Expand Down
8 changes: 2 additions & 6 deletions src/detect-bytejump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,6 @@ static DetectBytejumpData *DetectBytejumpParse(

static int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, const char *optstr)
{
SigMatch *sm = NULL;
SigMatch *prev_pm = NULL;
DetectBytejumpData *data = NULL;
char *offset = NULL;
Expand Down Expand Up @@ -569,12 +568,9 @@ static int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, const char
offset = NULL;
}

sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_BYTEJUMP, (SigMatchCtx *)data, sm_list) == NULL) {
goto error;
sm->type = DETECT_BYTEJUMP;
sm->ctx = (SigMatchCtx *)data;
SigMatchAppendSMToList(s, sm, sm_list);
}

if (!(data->flags & DETECT_BYTEJUMP_RELATIVE))
goto okay;
Expand Down
8 changes: 2 additions & 6 deletions src/detect-bytemath.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ static DetectByteMathData *DetectByteMathParse(
*/
static int DetectByteMathSetup(DetectEngineCtx *de_ctx, Signature *s, const char *arg)
{
SigMatch *sm = NULL;
SigMatch *prev_pm = NULL;
DetectByteMathData *data;
char *rvalue = NULL;
Expand Down Expand Up @@ -393,12 +392,9 @@ static int DetectByteMathSetup(DetectEngineCtx *de_ctx, Signature *s, const char
de_ctx->byte_extract_max_local_id = data->local_id;
}

sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_BYTEMATH, (SigMatchCtx *)data, sm_list) == NULL) {
goto error;
sm->type = DETECT_BYTEMATH;
sm->ctx = (void *)data;
SigMatchAppendSMToList(s, sm, sm_list);
}

if (!(data->flags & DETECT_BYTEMATH_FLAG_RELATIVE))
goto okay;
Expand Down
8 changes: 2 additions & 6 deletions src/detect-bytetest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,6 @@ static DetectBytetestData *DetectBytetestParse(

static int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, const char *optstr)
{
SigMatch *sm = NULL;
SigMatch *prev_pm = NULL;
char *value = NULL;
char *offset = NULL;
Expand Down Expand Up @@ -696,12 +695,9 @@ static int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, const char
nbytes = NULL;
}

sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_BYTETEST, (SigMatchCtx *)data, sm_list) == NULL) {
goto error;
sm->type = DETECT_BYTETEST;
sm->ctx = (SigMatchCtx *)data;
SigMatchAppendSMToList(s, sm, sm_list);
}

if (!(data->flags & DETECT_BYTETEST_RELATIVE))
goto okay;
Expand Down
26 changes: 6 additions & 20 deletions src/detect-cipservice.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ static int DetectCipServiceSetup(DetectEngineCtx *de_ctx, Signature *s,
SCEnter();

DetectCipServiceData *cipserviced = NULL;
SigMatch *sm = NULL;

if (DetectSignatureSetAppProto(s, ALPROTO_ENIP) != 0)
return -1;
Expand All @@ -217,21 +216,15 @@ static int DetectCipServiceSetup(DetectEngineCtx *de_ctx, Signature *s,
if (cipserviced == NULL)
goto error;

sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_CIPSERVICE, (SigMatchCtx *)cipserviced,
g_cip_buffer_id) == NULL) {
goto error;

sm->type = DETECT_CIPSERVICE;
sm->ctx = (void *) cipserviced;

SigMatchAppendSMToList(s, sm, g_cip_buffer_id);
}
SCReturnInt(0);

error:
if (cipserviced != NULL)
DetectCipServiceFree(de_ctx, cipserviced);
if (sm != NULL)
SCFree(sm);
SCReturnInt(-1);
}

Expand Down Expand Up @@ -378,7 +371,6 @@ static int DetectEnipCommandSetup(DetectEngineCtx *de_ctx, Signature *s,
const char *rulestr)
{
DetectEnipCommandData *enipcmdd = NULL;
SigMatch *sm = NULL;

if (DetectSignatureSetAppProto(s, ALPROTO_ENIP) != 0)
return -1;
Expand All @@ -387,21 +379,15 @@ static int DetectEnipCommandSetup(DetectEngineCtx *de_ctx, Signature *s,
if (enipcmdd == NULL)
goto error;

sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(
de_ctx, s, DETECT_ENIPCOMMAND, (SigMatchCtx *)enipcmdd, g_enip_buffer_id) == NULL) {
goto error;

sm->type = DETECT_ENIPCOMMAND;
sm->ctx = (void *) enipcmdd;

SigMatchAppendSMToList(s, sm, g_enip_buffer_id);
}
SCReturnInt(0);

error:
if (enipcmdd != NULL)
DetectEnipCommandFree(de_ctx, enipcmdd);
if (sm != NULL)
SCFree(sm);
SCReturnInt(-1);
}

Expand Down
13 changes: 4 additions & 9 deletions src/detect-config.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
SCEnter();

DetectConfigData *fd = NULL;
SigMatch *sm = NULL;
int res = 0;
size_t pcre2len;
#if 0
Expand All @@ -182,10 +181,6 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
}
#endif
pcre2_match_data *match = NULL;
sm = SigMatchAlloc();
if (sm == NULL)
goto error;
sm->type = DETECT_CONFIG;

if (str == NULL || strlen(str) == 0) {
SCLogError("config keywords need arguments");
Expand Down Expand Up @@ -297,8 +292,10 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
s->flags |= SIG_FLAG_APPLAYER;
}

sm->ctx = (SigMatchCtx*)fd;
SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_POSTMATCH);
if (SigMatchAppendSMToList(
de_ctx, s, DETECT_CONFIG, (SigMatchCtx *)fd, DETECT_SM_LIST_POSTMATCH) == NULL) {
goto error;
}

pcre2_match_data_free(match);
return 0;
Expand All @@ -307,8 +304,6 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
if (match) {
pcre2_match_data_free(match);
}
if (sm != NULL)
SCFree(sm);
return -1;
}

Expand Down
7 changes: 2 additions & 5 deletions src/detect-content.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,12 +353,9 @@ int DetectContentSetup(DetectEngineCtx *de_ctx, Signature *s, const char *conten
}
}

SigMatch *sm = SigMatchAlloc();
if (sm == NULL)
if (SigMatchAppendSMToList(de_ctx, s, DETECT_CONTENT, (SigMatchCtx *)cd, sm_list) == NULL) {
goto error;
sm->ctx = (void *)cd;
sm->type = DETECT_CONTENT;
SigMatchAppendSMToList(s, sm, sm_list);
}

return 0;

Expand Down
Loading

0 comments on commit c272a64

Please sign in to comment.