Skip to content

Commit

Permalink
detect/pcre: Use local match variables
Browse files Browse the repository at this point in the history 
pcre2 is not thread-safe wrt match objects so use locally scoped
objects.

Issue: 4797
  • Loading branch information
@jlucovsky @victorjulien
jlucovsky authored and victorjulien committed Jul 17, 2023
1 parent 27aa35c commit 3286c3b
Show file tree
Hide file tree
Showing 52 changed files with 576 additions and 319 deletions.
14 changes: 7 additions & 7 deletions src/detect-base64-decode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,9 @@ static int DetectBase64DecodeParse(const char *str, uint32_t *bytes,
*offset = 0;
*relative = 0;
size_t pcre2_len;
pcre2_match_data *match = NULL;

pcre2_match_data *match = pcre2_match_data_create_from_pattern(decode_pcre.regex, NULL);
if (match == NULL)
goto error;

int pcre_rc = pcre2_match(decode_pcre.regex, (PCRE2_SPTR8)str, strlen(str), 0, 0, match, NULL);
int pcre_rc = DetectParsePcreExec(&decode_pcre, &match, str, 0, 0);
if (pcre_rc < 3) {
goto error;
}
Expand Down Expand Up @@ -164,10 +161,13 @@ static int DetectBase64DecodeParse(const char *str, uint32_t *bytes,
}
}

retval = 1;

pcre2_match_data_free(match);
match = NULL;
retval = 1;

error:

if (bytes_str != NULL) {
pcre2_substring_free((PCRE2_UCHAR8 *)bytes_str);
}
Expand All @@ -177,7 +177,7 @@ static int DetectBase64DecodeParse(const char *str, uint32_t *bytes,
if (relative_str != NULL) {
pcre2_substring_free((PCRE2_UCHAR8 *)relative_str);
}
if (match != NULL) {
if (match) {
pcre2_match_data_free(match);
}
return retval;
Expand Down
26 changes: 14 additions & 12 deletions src/detect-byte-extract.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,12 @@ int DetectByteExtractDoMatch(DetectEngineThreadCtx *det_ctx, const SigMatchData
static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_ctx, const char *arg)
{
DetectByteExtractData *bed = NULL;
int ret = 0, res = 0;
int res = 0;
size_t pcre2len;
int i = 0;
pcre2_match_data *match = NULL;

ret = DetectParsePcreExec(&parse_regex, arg, 0, 0);
int ret = DetectParsePcreExec(&parse_regex, &match, arg, 0, 0);
if (ret < 3 || ret > 19) {
SCLogError("parse error, ret %" PRId32 ", string \"%s\"", ret, arg);
SCLogError("Invalid arg to byte_extract : %s "
Expand All @@ -235,8 +236,7 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
/* no of bytes to extract */
char nbytes_str[64] = "";
pcre2len = sizeof(nbytes_str);
res = pcre2_substring_copy_bynumber(
parse_regex.match, 1, (PCRE2_UCHAR8 *)nbytes_str, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 1, (PCRE2_UCHAR8 *)nbytes_str, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed "
"for arg 1 for byte_extract");
Expand All @@ -253,8 +253,7 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
/* offset */
char offset_str[64] = "";
pcre2len = sizeof(offset_str);
res = pcre2_substring_copy_bynumber(
parse_regex.match, 2, (PCRE2_UCHAR8 *)offset_str, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 2, (PCRE2_UCHAR8 *)offset_str, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed "
"for arg 2 for byte_extract");
Expand All @@ -270,8 +269,7 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
/* var name */
char varname_str[256] = "";
pcre2len = sizeof(varname_str);
res = pcre2_substring_copy_bynumber(
parse_regex.match, 3, (PCRE2_UCHAR8 *)varname_str, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 3, (PCRE2_UCHAR8 *)varname_str, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed "
"for arg 3 for byte_extract");
Expand All @@ -285,7 +283,7 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
for (i = 4; i < ret; i++) {
char opt_str[64] = "";
pcre2len = sizeof(opt_str);
res = SC_Pcre2SubstringCopy(parse_regex.match, i, (PCRE2_UCHAR8 *)opt_str, &pcre2len);
res = SC_Pcre2SubstringCopy(match, i, (PCRE2_UCHAR8 *)opt_str, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed "
"for arg %d for byte_extract with %d",
Expand All @@ -312,7 +310,7 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
char multiplier_str[16] = "";
pcre2len = sizeof(multiplier_str);
res = pcre2_substring_copy_bynumber(
parse_regex.match, i, (PCRE2_UCHAR8 *)multiplier_str, &pcre2len);
match, i, (PCRE2_UCHAR8 *)multiplier_str, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed "
"for arg %d for byte_extract",
Expand Down Expand Up @@ -416,8 +414,7 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_

char align_str[16] = "";
pcre2len = sizeof(align_str);
res = pcre2_substring_copy_bynumber(
parse_regex.match, i, (PCRE2_UCHAR8 *)align_str, &pcre2len);
res = pcre2_substring_copy_bynumber(match, i, (PCRE2_UCHAR8 *)align_str, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed "
"for arg %d in byte_extract",
Expand Down Expand Up @@ -507,10 +504,15 @@ static inline DetectByteExtractData *DetectByteExtractParse(DetectEngineCtx *de_
bed->endian = DETECT_BYTE_EXTRACT_ENDIAN_DEFAULT;
}

pcre2_match_data_free(match);

return bed;
error:
if (bed != NULL)
DetectByteExtractFree(de_ctx, bed);
if (match) {
pcre2_match_data_free(match);
}
return NULL;
}

Expand Down
14 changes: 9 additions & 5 deletions src/detect-bytejump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,18 +372,19 @@ static DetectBytejumpData *DetectBytejumpParse(
{
DetectBytejumpData *data = NULL;
char args[10][64];
int ret = 0, res = 0;
int res = 0;
size_t pcre2len;
int numargs = 0;
int i = 0;
uint32_t nbytes = 0;
char *str_ptr;
char *end_ptr;
pcre2_match_data *match = NULL;

memset(args, 0x00, sizeof(args));

/* Execute the regex and populate args with captures. */
ret = DetectParsePcreExec(&parse_regex, optstr, 0, 0);
int ret = DetectParsePcreExec(&parse_regex, &match, optstr, 0, 0);
if (ret < 2 || ret > 10) {
SCLogError("parse error, ret %" PRId32 ", string \"%s\"", ret, optstr);
goto error;
Expand All @@ -395,7 +396,7 @@ static DetectBytejumpData *DetectBytejumpParse(
*/
char str[512] = "";
pcre2len = sizeof(str);
res = pcre2_substring_copy_bynumber(parse_regex.match, 1, (PCRE2_UCHAR8 *)str, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 1, (PCRE2_UCHAR8 *)str, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed "
"for arg 1");
Expand Down Expand Up @@ -425,8 +426,7 @@ static DetectBytejumpData *DetectBytejumpParse(
/* The remaining args are directly from PCRE substrings */
for (i = 1; i < (ret - 1); i++) {
pcre2len = sizeof(args[0]);
res = pcre2_substring_copy_bynumber(
parse_regex.match, i + 1, (PCRE2_UCHAR8 *)args[i + 1], &pcre2len);
res = pcre2_substring_copy_bynumber(match, i + 1, (PCRE2_UCHAR8 *)args[i + 1], &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed for arg %d", i + 1);
goto error;
Expand Down Expand Up @@ -554,6 +554,7 @@ static DetectBytejumpData *DetectBytejumpParse(
}
}

pcre2_match_data_free(match);
return data;

error:
Expand All @@ -567,6 +568,9 @@ static DetectBytejumpData *DetectBytejumpParse(
}
if (data != NULL)
DetectBytejumpFree(de_ctx, data);
if (match) {
pcre2_match_data_free(match);
}
return NULL;
}

Expand Down
12 changes: 8 additions & 4 deletions src/detect-bytetest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,23 +330,23 @@ static DetectBytetestData *DetectBytetestParse(
};
char *test_value = NULL;
char *data_offset = NULL;
int ret = 0, res = 0;
int res = 0;
size_t pcre2_len;
int i;
uint32_t nbytes;
const char *str_ptr = NULL;
pcre2_match_data *match = NULL;

/* Execute the regex and populate args with captures. */
ret = DetectParsePcreExec(&parse_regex, optstr, 0, 0);
int ret = DetectParsePcreExec(&parse_regex, &match, optstr, 0, 0);
if (ret < 4 || ret > 9) {
SCLogError("parse error, ret %" PRId32 ", string %s", ret, optstr);
goto error;
}

/* Subtract two since two values are conjoined */
for (i = 0; i < (ret - 1); i++) {
res = pcre2_substring_get_bynumber(
parse_regex.match, i + 1, (PCRE2_UCHAR8 **)&str_ptr, &pcre2_len);
res = pcre2_substring_get_bynumber(match, i + 1, (PCRE2_UCHAR8 **)&str_ptr, &pcre2_len);
if (res < 0) {
SCLogError("pcre2_substring_get_bynumber failed "
"for arg %d",
Expand Down Expand Up @@ -562,6 +562,7 @@ static DetectBytetestData *DetectBytetestParse(
if (data_offset) SCFree(data_offset);
if (test_value)
pcre2_substring_free((PCRE2_UCHAR8 *)test_value);
pcre2_match_data_free(match);
return data;

error:
Expand All @@ -573,6 +574,9 @@ static DetectBytetestData *DetectBytetestParse(
if (test_value)
pcre2_substring_free((PCRE2_UCHAR8 *)test_value);
if (data) SCFree(data);
if (match) {
pcre2_match_data_free(match);
}
return NULL;
}

Expand Down
18 changes: 13 additions & 5 deletions src/detect-classtype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,27 +73,35 @@ static int DetectClasstypeParseRawString(const char *rawstr, char *out, size_t o

const size_t esize = CLASSTYPE_NAME_MAX_LEN + 8;
char e[esize];
pcre2_match_data *match = NULL;

int ret = DetectParsePcreExec(&parse_regex, rawstr, 0, 0);
int ret = DetectParsePcreExec(&parse_regex, &match, rawstr, 0, 0);
if (ret < 0) {
SCLogError("Invalid Classtype in Signature");
return -1;
goto error;
}

pcre2len = esize;
ret = pcre2_substring_copy_bynumber(parse_regex.match, 1, (PCRE2_UCHAR8 *)e, &pcre2len);
ret = pcre2_substring_copy_bynumber(match, 1, (PCRE2_UCHAR8 *)e, &pcre2len);
if (ret < 0) {
SCLogError("pcre2_substring_copy_bynumber failed");
return -1;
goto error;
}

if (strlen(e) >= CLASSTYPE_NAME_MAX_LEN) {
SCLogError("classtype '%s' is too big: max %d", rawstr, CLASSTYPE_NAME_MAX_LEN - 1);
return -1;
goto error;
}
(void)strlcpy(out, e, outsize);

pcre2_match_data_free(match);
return 0;

error:
if (match) {
pcre2_match_data_free(match);
}
return -1;
}

/**
Expand Down
21 changes: 13 additions & 8 deletions src/detect-config.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char

DetectConfigData *fd = NULL;
SigMatch *sm = NULL;
int ret = 0, res = 0;
int res = 0;
size_t pcre2len;
#if 0
/* filestore and bypass keywords can't work together */
Expand All @@ -181,6 +181,7 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
return -1;
}
#endif
pcre2_match_data *match = NULL;
sm = SigMatchAlloc();
if (sm == NULL)
goto error;
Expand All @@ -198,13 +199,13 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
char scopeval[32];
SCLogDebug("str %s", str);

ret = DetectParsePcreExec(&parse_regex, str, 0, 0);
int ret = DetectParsePcreExec(&parse_regex, &match, str, 0, 0);
if (ret != 7) {
SCLogError("config is rather picky at this time");
goto error;
}
pcre2len = sizeof(subsys);
res = pcre2_substring_copy_bynumber(parse_regex.match, 1, (PCRE2_UCHAR8 *)subsys, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 1, (PCRE2_UCHAR8 *)subsys, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed");
goto error;
Expand All @@ -217,7 +218,7 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
SCLogDebug("subsys %s", subsys);

pcre2len = sizeof(state);
res = pcre2_substring_copy_bynumber(parse_regex.match, 2, (PCRE2_UCHAR8 *)state, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 2, (PCRE2_UCHAR8 *)state, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed");
goto error;
Expand All @@ -230,7 +231,7 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
SCLogDebug("state %s", state);

pcre2len = sizeof(type);
res = pcre2_substring_copy_bynumber(parse_regex.match, 3, (PCRE2_UCHAR8 *)type, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 3, (PCRE2_UCHAR8 *)type, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed");
goto error;
Expand All @@ -243,7 +244,7 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
SCLogDebug("type %s", type);

pcre2len = sizeof(typeval);
res = pcre2_substring_copy_bynumber(parse_regex.match, 4, (PCRE2_UCHAR8 *)typeval, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 4, (PCRE2_UCHAR8 *)typeval, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed");
goto error;
Expand All @@ -256,7 +257,7 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
SCLogDebug("typeval %s", typeval);

pcre2len = sizeof(scope);
res = pcre2_substring_copy_bynumber(parse_regex.match, 5, (PCRE2_UCHAR8 *)scope, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 5, (PCRE2_UCHAR8 *)scope, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed");
goto error;
Expand All @@ -269,7 +270,7 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
SCLogDebug("scope %s", scope);

pcre2len = sizeof(scopeval);
res = pcre2_substring_copy_bynumber(parse_regex.match, 6, (PCRE2_UCHAR8 *)scopeval, &pcre2len);
res = pcre2_substring_copy_bynumber(match, 6, (PCRE2_UCHAR8 *)scopeval, &pcre2len);
if (res < 0) {
SCLogError("pcre2_substring_copy_bynumber failed");
goto error;
Expand Down Expand Up @@ -299,9 +300,13 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
sm->ctx = (SigMatchCtx*)fd;
SigMatchAppendSMToList(s, sm, DETECT_SM_LIST_POSTMATCH);

pcre2_match_data_free(match);
return 0;

error:
if (match) {
pcre2_match_data_free(match);
}
if (sm != NULL)
SCFree(sm);
return -1;
Expand Down
Loading

0 comments on commit 3286c3b

Please sign in to comment.