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

detect/pcre: fix memory read error in detect

Browse Source 
Fix case where a HTTP modifier in PCRE statements in a rule that would not
set the http protocol, would lead to a HTTP condition being run against
a non-HTTP flow. This would lead to invalid memory access.

Fix by properly setting the alproto and SIG_FLAG_APPLAYER flag in the
signature, leading to the signature implicitly setting the protocol
so rejecting it for inspection when the flow has a different protocol.

Bug #2863
pull/4329/head
Victor Julien 6 years ago
parent 61eb9c21ed
commit 17295591ee
1 changed files with 59 additions and 19 deletions
Show Stats Download Patch File Download Diff File

78
src/detect-pcre.c
Unescape Escape View File

@ -324,8 +324,9 @@ static int DetectPcreHasUpperCase(const char *re)
return 0;
}
static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *regexstr, int *sm_list,
char *capture_names, size_t capture_names_size, bool negate)
static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx,
const char *regexstr, int *sm_list, char *capture_names,
size_t capture_names_size, bool negate, AppProto *alproto)
{
int ec;
const char *eb;
@ -466,6 +467,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
}
int list = DetectBufferTypeGetByName("http_uri");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'V': {
@ -475,6 +477,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
}
int list = DetectBufferTypeGetByName("http_user_agent");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'W': {
@ -484,6 +487,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
}
int list = DetectBufferTypeGetByName("http_host");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
check_host_header = 1;
break;
}
@ -494,6 +498,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
}
int list = DetectBufferTypeGetByName("http_raw_host");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'H': { /* snort's option */
@ -503,6 +508,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
}
int list = DetectBufferTypeGetByName("http_header");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
} case 'I': { /* snort's option */
if (pd->flags & DETECT_PCRE_RAWBYTES) {
@ -511,11 +517,13 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
}
int list = DetectBufferTypeGetByName("http_raw_uri");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'D': { /* snort's option */
int list = DetectBufferTypeGetByName("http_raw_header");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'M': { /* snort's option */
@ -525,6 +533,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
}
int list = DetectBufferTypeGetByName("http_method");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'C': { /* snort's option */
@ -534,30 +543,35 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
}
int list = DetectBufferTypeGetByName("http_cookie");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'P': {
/* snort's option (http request body inspection) */
int list = DetectBufferTypeGetByName("http_client_body");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'Q': {
int list = DetectBufferTypeGetByName("file_data");
/* suricata extension (http response body inspection) */
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'Y': {
/* snort's option */
int list = DetectBufferTypeGetByName("http_stat_msg");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
case 'S': {
/* snort's option */
int list = DetectBufferTypeGetByName("http_stat_code");
*sm_list = DetectPcreSetList(*sm_list, list);
*alproto = ALPROTO_HTTP;
break;
}
default:
@ -661,7 +675,6 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
} else {
goto error;
}
return pd;
error:
@ -817,9 +830,11 @@ static int DetectPcreSetup (DetectEngineCtx *de_ctx, Signature *s, const char *r
int ret = -1;
int parsed_sm_list = DETECT_SM_LIST_NOTSET;
char capture_names[1024] = "";
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, regexstr, &parsed_sm_list,
capture_names, sizeof(capture_names), s->init_data->negated);
capture_names, sizeof(capture_names), s->init_data->negated,
&alproto);
if (pd == NULL)
goto error;
if (DetectPcreParseCapture(regexstr, de_ctx, pd, capture_names) < 0)
@ -834,9 +849,19 @@ static int DetectPcreSetup (DetectEngineCtx *de_ctx, Signature *s, const char *r
case DETECT_SM_LIST_NOTSET:
sm_list = DETECT_SM_LIST_PMATCH;
break;
default:
default: {
if (alproto != ALPROTO_UNKNOWN) {
/* see if the proto doesn't conflict
* with what we already have. */
if (s->alproto != ALPROTO_UNKNOWN &&
alproto != s->alproto) {
goto error;
}
DetectSignatureSetAppProto(s, alproto);
}
sm_list = parsed_sm_list;
break;
}
}
}
if (sm_list == -1)
@ -918,8 +943,9 @@ static int DetectPcreParseTest01 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NOT_NULL(pd);
DetectEngineCtxFree(de_ctx);
@ -937,9 +963,11 @@ static int DetectPcreParseTest02 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NOT_NULL(pd);
FAIL_IF_NOT(alproto == ALPROTO_HTTP);
DetectEngineCtxFree(de_ctx);
return result;
@ -956,8 +984,9 @@ static int DetectPcreParseTest03 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NOT_NULL(pd);
DetectEngineCtxFree(de_ctx);
@ -975,9 +1004,11 @@ static int DetectPcreParseTest04 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NULL(pd);
FAIL_IF_NOT(alproto == ALPROTO_UNKNOWN);
DetectPcreFree(pd);
DetectEngineCtxFree(de_ctx);
@ -995,9 +1026,11 @@ static int DetectPcreParseTest05 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NULL(pd);
FAIL_IF_NOT(alproto == ALPROTO_UNKNOWN);
DetectPcreFree(pd);
DetectEngineCtxFree(de_ctx);
@ -1015,9 +1048,11 @@ static int DetectPcreParseTest06 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NULL(pd);
FAIL_IF_NOT(alproto == ALPROTO_UNKNOWN);
DetectPcreFree(pd);
DetectEngineCtxFree(de_ctx);
@ -1035,9 +1070,11 @@ static int DetectPcreParseTest07 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NULL(pd);
FAIL_IF_NOT(alproto == ALPROTO_HTTP);
DetectPcreFree(pd);
DetectEngineCtxFree(de_ctx);
@ -1055,9 +1092,11 @@ static int DetectPcreParseTest08 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NULL(pd);
FAIL_IF_NOT(alproto == ALPROTO_UNKNOWN);
DetectPcreFree(pd);
DetectEngineCtxFree(de_ctx);
@ -1075,8 +1114,9 @@ static int DetectPcreParseTest09 (void)
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
AppProto alproto = ALPROTO_UNKNOWN;
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
FAIL_IF_NULL(pd);
DetectPcreFree(pd);
@ -3419,30 +3459,30 @@ static int DetectPcreFlowvarCapture03(void)
*/
static int DetectPcreParseHttpHost(void)
{
DetectPcreData *pd = NULL;
AppProto alproto = ALPROTO_UNKNOWN;
int list = DETECT_SM_LIST_NOTSET;
DetectEngineCtx *de_ctx = DetectEngineCtxInit();
FAIL_IF(de_ctx == NULL);
pd = DetectPcreParse(de_ctx, "/domain\\.com/W", &list, NULL, 0, false);
DetectPcreData *pd = DetectPcreParse(de_ctx, "/domain\\.com/W", &list, NULL, 0, false, &alproto);
FAIL_IF(pd == NULL);
DetectPcreFree(pd);
list = DETECT_SM_LIST_NOTSET;
pd = DetectPcreParse(de_ctx, "/dOmain\\.com/W", &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, "/dOmain\\.com/W", &list, NULL, 0, false, &alproto);
FAIL_IF(pd != NULL);
/* Uppercase meta characters are valid. */
list = DETECT_SM_LIST_NOTSET;
pd = DetectPcreParse(de_ctx, "/domain\\D+\\.com/W", &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, "/domain\\D+\\.com/W", &list, NULL, 0, false, &alproto);
FAIL_IF(pd == NULL);
DetectPcreFree(pd);
/* This should not parse as the first \ escapes the second \, then
* we have a D. */
list = DETECT_SM_LIST_NOTSET;
pd = DetectPcreParse(de_ctx, "/\\\\Ddomain\\.com/W", &list, NULL, 0, false);
pd = DetectPcreParse(de_ctx, "/\\\\Ddomain\\.com/W", &list, NULL, 0, false, &alproto);
FAIL_IF(pd != NULL);
DetectEngineCtxFree(de_ctx);

Write Preview
Loading…
Cancel
Save