Skip to content

fix: handle regex errors of @validateHash like @rx #3712

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

Merged
airween merged 2 commits into from
Oct 27, 2026
Merged

fix: handle regex errors of @validateHash like @rx #3712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49f9ded
fix: handle regex errors of @validateHash like @rx
airween Oct 26, 2026
eeeb294
fix: set TX:MSC_PCRE_ERROR on exceeded PCRE limits too
airween Oct 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 68 additions & 70 deletions apache2/re_operators.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,58 @@
return 1; /* OK */
}

/* Sets a TX:MSC_* flag to 1, where TX:/^MSC_/ rules see it. */
static int msre_op_set_tx_flag(modsec_rec *msr, const char *name) {
msc_string *s = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));

if (s == NULL) return -1;
s->name = apr_pstrdup(msr->mp, name);
if (s->name == NULL) return -1;
s->name_len = strlen(s->name);
s->value = apr_pstrdup(msr->mp, "1");
if (s->value == NULL) return -1;
s->value_len = 1;
apr_table_setn(msr->tx_vars, s->name, (void *)s);

return 1;
}

/* Tells whether a regex execution failed on a PCRE limit. */
static int msre_op_pcre_limits_exceeded(int rc) {
#ifndef WITH_PCRE
return (rc == PCRE2_ERROR_MATCHLIMIT) || (rc == PCRE2_ERROR_RECURSIONLIMIT);
#else
return (rc == PCRE_ERROR_MATCHLIMIT) || (rc == PCRE_ERROR_RECURSIONLIMIT)
#ifdef PCRE_ERROR_JITSTACKLIMIT
/* Unlike msc_regexec_ex() with PCRE2, pcre_exec() does not retry
* without JIT when the JIT stack is full. */
|| (rc == PCRE_ERROR_JITSTACKLIMIT)
#endif
;
#endif
}

/* Flags and logs a regex execution that failed on a PCRE limit. A limit
* error is a regex error too, so both TX:MSC_PCRE_LIMITS_EXCEEDED and
* TX:MSC_PCRE_ERROR are set, the more specific one first. */
static int msre_op_pcre_limits_error(modsec_rec *msr, msre_rule *rule, int rc,
const char *my_error_msg, char **error_msg) {
if (msre_op_set_tx_flag(msr, "MSC_PCRE_LIMITS_EXCEEDED") < 0) return -1;
if (msre_op_set_tx_flag(msr, "MSC_PCRE_ERROR") < 0) return -1;

*error_msg = apr_psprintf(msr->mp,
"Rule %pp [id \"%s\"][file \"%s\"][line \"%d\"] - "
"Execution error - "
"PCRE limits exceeded (%d): %s",
rule,((rule->actionset != NULL)&&(rule->actionset->id != NULL)) ? rule->actionset->id : "-",
rule->filename != NULL ? rule->filename : "-",
rule->line_num,rc, my_error_msg);

msr_log(msr, 3, "%s.", *error_msg);

return 1;
}

/**
* \brief Execute function to validateHash
*
Expand All @@ -784,7 +836,7 @@
* \retval 1 On success
* \retval 0 On fail
*/
static int msre_op_validateHash_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, char **error_msg) {

Check failure on line 839 in apache2/re_operators.c

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 72 to the 25 allowed. 


See more on https://sonarcloud.io/project/issues?id=owasp-modsecurity_ModSecurity&issues=AaF54iOE2c1245phkBVF&open=AaF54iOE2c1245phkBVF&pullRequest=3712
assert(msr != NULL);
assert(rule != NULL);
assert(var != NULL);
Expand Down Expand Up @@ -845,7 +897,12 @@
if (regex == NULL) {
*error_msg = apr_psprintf(msr->mp, "Error compiling pattern (offset %d): %s",
erroffset, errptr);
return 0;
msr_log(msr, 3, "%s.", *error_msg);
if (msre_op_set_tx_flag(msr, "MSC_PCRE_ERROR") < 0) return -1;

Check failure on line 901 in apache2/re_operators.c

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this code to not nest more than 3 if|for|do|while|switch statements. 


See more on https://sonarcloud.io/project/issues?id=owasp-modsecurity_ModSecurity&issues=AaF54iOF2c1245phkBVG&open=AaF54iOF2c1245phkBVG&pullRequest=3712

/* A regex error is neither a match nor a mismatch: return
* what the negation in re.c turns into "no match". */
return rule->op_negated;
}

#ifdef WITH_PCRE_STUDY
Expand Down Expand Up @@ -889,37 +946,17 @@
* and no memory has to be allocated for any backreferences.
*/
rc = msc_regexec_capture(regex, target, target_length, ovector, 30, &my_error_msg);
#ifndef WITH_PCRE
if ((rc == PCRE2_ERROR_MATCHLIMIT) || (rc == PCRE2_ERROR_RECURSIONLIMIT)) {
#else
if ((rc == PCRE_ERROR_MATCHLIMIT) || (rc == PCRE_ERROR_RECURSIONLIMIT)) {
#endif
msc_string *s = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));

if (s == NULL) return -1;
s->name = apr_pstrdup(msr->mp, "MSC_PCRE_LIMITS_EXCEEDED");
if (s->name == NULL) return -1;
s->name_len = strlen(s->name);
s->value = apr_pstrdup(msr->mp, "1");
if (s->value == NULL) return -1;
s->value_len = 1;
apr_table_setn(msr->tx_vars, s->name, (void *)s);
if (msre_op_pcre_limits_exceeded(rc)) {
if (msre_op_pcre_limits_error(msr, rule, rc, my_error_msg, error_msg) < 0) return -1;

*error_msg = apr_psprintf(msr->mp,
"Rule %pp [id \"%s\"][file \"%s\"][line \"%d\"] - "
"Execution error - "
"PCRE limits exceeded (%d): %s",
rule,((rule->actionset != NULL)&&(rule->actionset->id != NULL)) ? rule->actionset->id : "-",
rule->filename != NULL ? rule->filename : "-",
rule->line_num,rc, my_error_msg);

msr_log(msr, 3, "%s.", *error_msg);

return 0; /* No match. */
/* A regex error is neither a match nor a mismatch: return
* what the negation in re.c turns into "no match". */
return rule->op_negated;
}
else if (rc < -1) {
*error_msg = apr_psprintf(msr->mp, "Regex execution failed (%d): %s",
rc, my_error_msg);
msre_op_set_tx_flag(msr, "MSC_PCRE_ERROR");
return -1;
}

Expand Down Expand Up @@ -1050,23 +1087,7 @@
return 1; /* OK */
}

/* Flags a regex error in TX:MSC_PCRE_ERROR, where TX:/^MSC_/ rules see it. */
static int msre_op_rx_set_error_flag(modsec_rec *msr) {
msc_string *s = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));

if (s == NULL) return -1;
s->name = apr_pstrdup(msr->mp, "MSC_PCRE_ERROR");
if (s->name == NULL) return -1;
s->name_len = strlen(s->name);
s->value = apr_pstrdup(msr->mp, "1");
if (s->value == NULL) return -1;
s->value_len = 1;
apr_table_setn(msr->tx_vars, s->name, (void *)s);

return 1;
}

static int msre_op_rx_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, char **error_msg) {

Check failure on line 1090 in apache2/re_operators.c

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 110 to the 25 allowed. 


See more on https://sonarcloud.io/project/issues?id=owasp-modsecurity_ModSecurity&issues=AaF54iOF2c1245phkBVH&open=AaF54iOF2c1245phkBVH&pullRequest=3712
assert(msr != NULL);
assert(rule != NULL);
assert(rule->actionset != NULL);
Expand Down Expand Up @@ -1131,7 +1152,7 @@
*error_msg = apr_psprintf(msr->mp, "Error compiling pattern (offset %d): %s",
erroffset, errptr);
msr_log(msr, 3, "%s.", *error_msg);
if (msre_op_rx_set_error_flag(msr) < 0) return -1;
if (msre_op_set_tx_flag(msr, "MSC_PCRE_ERROR") < 0) return -1;

Check failure on line 1155 in apache2/re_operators.c

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this code to not nest more than 3 if|for|do|while|switch statements. 


See more on https://sonarcloud.io/project/issues?id=owasp-modsecurity_ModSecurity&issues=AaF54iOF2c1245phkBVI&open=AaF54iOF2c1245phkBVI&pullRequest=3712

/* A regex error is neither a match nor a mismatch: return
* what the negation in re.c turns into "no match". */
Expand Down Expand Up @@ -1209,31 +1230,8 @@
* and no memory has to be allocated for any backreferences.
*/
rc = msc_regexec_capture(regex, target, target_length, ovector, 30, &my_error_msg);
#ifndef WITH_PCRE
if ((rc == PCRE2_ERROR_MATCHLIMIT) || (rc == PCRE2_ERROR_RECURSIONLIMIT)) {
#else
if ((rc == PCRE_ERROR_MATCHLIMIT) || (rc == PCRE_ERROR_RECURSIONLIMIT)) {
#endif
msc_string *s = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));

if (s == NULL) return -1;
s->name = apr_pstrdup(msr->mp, "MSC_PCRE_LIMITS_EXCEEDED");
if (s->name == NULL) return -1;
s->name_len = strlen(s->name);
s->value = apr_pstrdup(msr->mp, "1");
if (s->value == NULL) return -1;
s->value_len = 1;
apr_table_setn(msr->tx_vars, s->name, (void *)s);

*error_msg = apr_psprintf(msr->mp,
"Rule %pp [id \"%s\"][file \"%s\"][line \"%d\"] - "
"Execution error - "
"PCRE limits exceeded (%d): %s",
rule,((rule->actionset != NULL)&&(rule->actionset->id != NULL)) ? rule->actionset->id : "-",
rule->filename != NULL ? rule->filename : "-",
rule->line_num,rc, my_error_msg);

msr_log(msr, 3, "%s.", *error_msg);
if (msre_op_pcre_limits_exceeded(rc)) {
if (msre_op_pcre_limits_error(msr, rule, rc, my_error_msg, error_msg) < 0) return -1;

/* A regex error is neither a match nor a mismatch: return
* what the negation in re.c turns into "no match". */
Expand All @@ -1242,7 +1240,7 @@
else if (rc < -1) {
*error_msg = apr_psprintf(msr->mp, "Regex execution failed (%d): %s",
rc, my_error_msg);
msre_op_rx_set_error_flag(msr);
msre_op_set_tx_flag(msr, "MSC_PCRE_ERROR");
return -1;
}

Expand Down
132 changes: 129 additions & 3 deletions tests/regression/misc/10-pcre-negated.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
### Regex errors with negated @rx
### Regex errors with negated @rx and @validateHash

# Neither @rx nor !@rx must match when the regex fails. A failure must
# be visible through the TX:MSC_* flags instead.
# Neither the operator nor its negation must match when the regex fails.
# A failure must be visible through the TX:MSC_* flags instead.

{
type => "misc",
Expand Down Expand Up @@ -147,3 +147,129 @@
GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html?x=a&patt=x%28",
),
},
{
type => "misc",
comment => "Invalid expanded pattern - negated validateHash does not match, MSC_PCRE_ERROR is set",
conf => qq(
SecRuleEngine On
SecDebugLog $ENV{DEBUG_LOG}
SecDebugLogLevel 9
SecHashEngine On

SecRule ARGS:x "!\@validateHash %{ARGS.patt}" "phase:2,deny,id:500068"
SecRule TX:MSC_PCRE_ERROR "\@streq 1" "phase:2,pass,log,msg:'PCRE error flagged',id:500069"
),
match_log => {
debug => [ qr/Error compiling pattern/, 1 ],
error => [ qr/PCRE error flagged/, 1 ],
-error => [ qr/Access denied/, 1 ],
},
match_response => {
status => qr/^200$/,
},
request => new HTTP::Request(
GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html?x=a&patt=%2A",
),
},
{
type => "misc",
comment => "PCRE limits exceeded - negated validateHash does not match",
conf => qq(
SecRuleEngine On
SecDebugLog $ENV{DEBUG_LOG}
SecDebugLogLevel 9
SecHashEngine On

SecRequestBodyAccess On
SecRequestBodyLimit 1000000
SecRequestBodyInMemoryLimit 1000000

SecPcreMatchLimit 100
SecPcreMatchLimitRecursion 100

SecRule ARGS "!\@validateHash (?:(.{2,})\\1{32,})" "phase:2,deny,id:500070"
SecRule TX:MSC_PCRE_LIMITS_EXCEEDED "\@streq 1" "phase:2,pass,log,msg:'PCRE limits flagged',id:500071"
),
match_log => {
debug => [ qr/PCRE limits exceeded/, 1 ],
error => [ qr/PCRE limits flagged/, 1 ],
-error => [ qr/Access denied/, 1 ],
},
match_response => {
status => qr/^200$/,
},
request => new HTTP::Request(
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html",
[
"Content-Type" => "application/x-www-form-urlencoded",
],
"test=" . (("a" x 64) . "b") x 64 . "d",
),
},
{
type => "misc",
comment => "PCRE limits exceeded - MSC_PCRE_ERROR is set too",
conf => qq(
SecRuleEngine On
SecDebugLog $ENV{DEBUG_LOG}
SecDebugLogLevel 9

SecRequestBodyAccess On
SecRequestBodyLimit 1000000
SecRequestBodyInMemoryLimit 1000000

SecPcreMatchLimit 100
SecPcreMatchLimitRecursion 100

SecRule ARGS "!\@rx (?:(.{2,})\\1{32,})" "phase:2,deny,id:500072"
SecRule TX:MSC_PCRE_ERROR "\@streq 1" "phase:2,pass,log,msg:'PCRE error flagged',id:500073"
),
match_log => {
debug => [ qr/PCRE limits exceeded/, 1 ],
error => [ qr/PCRE error flagged/, 1 ],
-error => [ qr/Access denied/, 1 ],
},
match_response => {
status => qr/^200$/,
},
request => new HTTP::Request(
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html",
[
"Content-Type" => "application/x-www-form-urlencoded",
],
"test=" . (("a" x 64) . "b") x 64 . "d",
),
},
{
type => "misc",
comment => "PCRE limits exceeded - TX:/^MSC_/ rule names MSC_PCRE_LIMITS_EXCEEDED",
conf => qq(
SecRuleEngine On
SecDebugLog $ENV{DEBUG_LOG}
SecDebugLogLevel 9

SecRequestBodyAccess On
SecRequestBodyLimit 1000000
SecRequestBodyInMemoryLimit 1000000

SecPcreMatchLimit 100
SecPcreMatchLimitRecursion 100

SecRule ARGS "!\@rx (?:(.{2,})\\1{32,})" "phase:2,deny,id:500074"
SecRule TX:/^MSC_/ "!\@streq 0" "phase:2,t:none,log,deny,msg:'ModSecurity internal error flagged: %{MATCHED_VAR_NAME}',id:500075"
),
match_log => {
error => [ qr/internal error flagged: TX:MSC_PCRE_LIMITS_EXCEEDED/, 1 ],
-error => [ qr/against "ARGS:test" required/, 1 ],
},
match_response => {
status => qr/^403$/,
},
request => new HTTP::Request(
POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html",
[
"Content-Type" => "application/x-www-form-urlencoded",
],
"test=" . (("a" x 64) . "b") x 64 . "d",
),
},
Loading