Allow regular expressions in ctl:ruleRemoveTargetByX variable names #911 #1683

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.

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

Jump to bottom

Closed

vvidic wants to merge 1 commit into owasp-modsecurity:v2/master from vvidic:ctl-regex

Closed

Allow regular expressions in ctl:ruleRemoveTargetByX variable names #911 #1683

vvidic wants to merge 1 commit into owasp-modsecurity:v2/master from vvidic:ctl-regex

Conversation

vvidic

Copy link

@vvidic vvidic commented Feb 23, 2018

SecRule REQUEST_URI "@beginswith /index.php"
"id:1001,phase:1,pass,nolog,
ctl:ruleRemoveTargetById=942100;ARGS:/^password[\d+]$/"


 Allow regular expressions in ctl:ruleRemoveTargetByX variable names o...

0bc30bd

...wasp-modsecurity#911
SecRule REQUEST_URI "@beginswith /index.php" \
 "id:1001,phase:1,pass,nolog, \
 ctl:ruleRemoveTargetById=942100;ARGS:/^password\[\d+\]$/"

@zimmerle zimmerle self-assigned this

Feb 23, 2018

@zimmerle zimmerle self-requested a review

February 23, 2018 22:00

@zimmerle zimmerle modified the milestones: v3.0.1, v2.9.3

Feb 23, 2018

@zimmerle zimmerle added the 2.x label

Feb 28, 2018

@zimmerle

Copy link

Contributor

zimmerle commented Feb 28, 2018

Hi @vvidic,

Thank you for the patch. As of the release of version 3 we are only merging new features if they are also available for v3.

@zimmerle zimmerle added the waiting for v3 label

Feb 28, 2018

@vvidic

Copy link

Author

vvidic commented Feb 28, 2018

So you mean I should send a patch for v3 branch than?

@zimmerle

Copy link

Contributor

zimmerle commented Feb 28, 2018

Hi @vvidic,

It means that it won't be released until we have the same functionality in v3.

@zimmerle zimmerle modified the milestones: v2.9.3, v2.9.4

Nov 2, 2018

@zimmerle zimmerle force-pushed the v2/master branch from 953f564 to 52532a1 Compare

December 15, 2018 02:48

@emphazer

Copy link

emphazer commented Jan 4, 2019

@vvidic thx a lot for this wonderful patch.
this makes modsec v2 much better and easier to handle on multi home servers

@fzipi

Copy link

Contributor

fzipi commented Jul 17, 2019

@vvidic Do you have the functionality available for v3?

@vvidic

Copy link

Author

vvidic commented Jul 17, 2019 via email

On Wed, Jul 17, 2019 at 10:38:29AM -0700, Felipe Zipitría wrote: @vvidic Do you have the functionality available for v3?

Nope, don't think I found the place where it should go. Let me know if you have any suggestions.

...

-- Valentin

@fzipi

Copy link

Contributor

azurit commented Aug 27, 2019

Should be merged even if there's no v3 version, some rules cannot be written without this feature.

@vvidic

Copy link

Author

vvidic commented Aug 27, 2019 via email

On Tue, Aug 27, 2019 at 07:40:03AM -0700, azurit wrote: Should be merged even if there's no v3 version, some rules cannot be written without this feature.

Right, but I think this was a request from the upstream authors.

...

-- Valentin

@azurit

Copy link

azurit commented Aug 27, 2019

I was talking to them off source :) btw, thnx for the patch.

@vvidic

Copy link

Author

vvidic commented Aug 27, 2019 via email

On Tue, Aug 27, 2019 at 10:44:47AM -0700, azurit wrote: I was talking to them off source :) btw, thnx for the patch.

Great :) In the meanwhile I checked the v3 code and found 2 places in src/rule.cc where RemoveTargetById/Tag is being used so this is probably where the changes should go...

...

-- Valentin

@azurit

Copy link

azurit commented Aug 29, 2019 •

edited

Loading

Example of exclusive rule which cannot be written without this feature (Typo3, probably a CSRF security token which is part of input name):

"Warning. Pattern match \"(?:;|\\\\{|\\\\||\\\\|\\\\||&|&&|\\\\n|\\\\r|\\\\$\\\$|\\\\$\\\\(\\\\(|`|\\\\${|<\\\\(|>\\\\(|\\\\(\\\\s*\\\$)\\\\s*(?:{|\\\\s*\\\$\\\\s*|\\\\w+=(?:[^\\\\s]*|\\\\$.*|\\\\$.*|<.*|>.*|\\\\'.*\\\\'|\\\".*\\\")\\\\s+|!\\\\s*|\\\\$)*\\\\s*(?:'|\\\")*(?:[\\\\?\\\\*\\\\[\\\\]\\\\(\\\$\\\\-\\\\|+\\\\w'\\\"\\\\./\\\\\\\\]+/)?[\\\\\\\\'\\\"]*(?:l[\\\\\\\\'\\\"]* ...\" at ARGS:data[tt_content][NEW5d67a8343a775100352544][bodytext]. [file \"/usr/share/modsecurity-crs/rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf\"] [line \"81\"] [id \"932100\"] [rev \"4\"] [msg \"Remote Command Execution: Unix Command Injection\"] [data \"Matched Data: ;\\x0d\\x0a(function found within ARGS:data[tt_content][NEW5d67a8343a775100352544][bodytext]: <style>\\x0d\\x0a #_form_52_ { font-size:14px; line-height:1.6; font-family:arial, helvetica, sans-serif; margin:0; }\\x0d\\x0a #_form_52_ * { outline:0; }\\x0d\\x0a ._form_hide { display:none; visibility:hidden; }\\x0d\\x0a ._form_show { display:block; visibility:visible; }\\x0d\\x0a #_form_52_._form-top { top:0; }\\x0d\\x0a #_form_52_._form-bottom { bottom:0; }\\x0d\\x0a #_form_52_._form-left { left"

@vvidic

Copy link

Author

vvidic commented Aug 29, 2019 via email

Another obstacle in the v3 implementation seems to be the parser, as it does not except the regex in the variable name: SecRule REQUEST_FILENAME "@ENDSWITH /wp-login.php" "id:9002100,phase:2,t:none,nolog,pass,ctl:ruleRemoveTargetById=123;ARGS:/^pwd$/" Rules error. File: action-ctl_rule_remove_target_by_id.json. Line: 1. Column: 132. Expecting an action, got: ^pwd$/" Not sure how to work around this?

...

-- Valentin

@azurit

Copy link

azurit commented Mar 12, 2020

@vvidic Today i came accross a bug in this feature: It's not possible to match a pipe symbol ( | ), even when escaped. Apache will print this error:
Error parsing actions: Unknown action: _]+\\]$/

Example rule (Prestashop web translation feature):

SecRule REQUEST_FILENAME "@rx /admin[0-9a-zA-Z]+/index\.php$" \
 "id:9990317,\
 phase:2,\
 pass,\
 t:none,\
 nolog,\
 chain"
 SecRule ARGS_GET:controller "@streq AdminTranslations" \
 "t:none,\
 chain"
 SecRule &ARGS_GET:controller "@eq 1" \
 "t:none,\
 ctl:ruleRemoveTargetById=932100;ARGS:/^core_mail\[txt\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=932105;ARGS:/^core_mail\[txt\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=932105;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=932100;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=921130;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=941180;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=932110;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=941100;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=941140;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=941260;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=941250;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=941190;ARGS:/^core_mail\[html\]\[[a-z_]+\]$/,\
 ctl:ruleRemoveTargetById=932100;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=921130;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=941180;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=932110;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=941100;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=941140;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=941190;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=941250;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=941260;ARGS:/^module_mail\[html\]\[[a-z0-9\-\|_]+\]$/,\
 ctl:ruleRemoveTargetById=930120;ARGS:/^[0-9a-z]+$/,\
 ctl:ruleRemoveTargetById=933180;ARGS:/^[0-9a-z]+$/"

The interesting is that escaped pipe symbol can be used in parts where regex is officially supported (e.g. in REQUEST_FILENAME matching in the beginning of the rule).

@fzipi fzipi mentioned this pull request

May 5, 2021

Wordpress - Preview Post - REQUEST-941-APPLICATION-ATTACK-XSS.conf coreruleset/coreruleset#2065

Closed

@azurit

Copy link

azurit commented Apr 16, 2024

Won't compile with --with-pcre2, modsec 2.9.7:

re.c:124:41: error: 'PCRE_DOTALL' undeclared (first use in this function); did you mean 'PCRE2_DOTALL'?
 PCRE_DOTALL | PCRE_CASELESS | PCRE_DOLLAR_ENDONLY, (const char **)&errptr, &erroffset);
 ^~~~~~~~~~~
 PCRE2_DOTALL
re.c:124:41: note: each undeclared identifier is reported only once for each function it appears in
re.c:124:55: error: 'PCRE_CASELESS' undeclared (first use in this function); did you mean 'PCRE2_CASELESS'?
 PCRE_DOTALL | PCRE_CASELESS | PCRE_DOLLAR_ENDONLY, (const char **)&errptr, &erroffset);
 ^~~~~~~~~~~~~
 PCRE2_CASELESS
re.c:124:71: error: 'PCRE_DOLLAR_ENDONLY' undeclared (first use in this function); did you mean 'PCRE2_DOLLAR_ENDONLY'?
 PCRE_DOTALL | PCRE_CASELESS | PCRE_DOLLAR_ENDONLY, (const char **)&errptr, &erroffset);
 ^~~~~~~~~~~~~~~~~~~
 PCRE2_DOLLAR_ENDONLY
re.c:131:96: error: 'PCRE_ERROR_NOMATCH' undeclared (first use in this function); did you mean 'PCRE2_ERROR_NOMATCH'?
 if (!(msc_regexec(regex, myvalue, strlen(myvalue), &errptr) == PCRE_ERROR_NOMATCH)) {
 ^~~~~~~~~~~~~~~~~~
 PCRE2_ERROR_NOMATCH

@airween

Copy link

Member

airween commented Apr 16, 2024

Won't compile with --with-pcre2, modsec 2.9.7:

Thanks, I'll take a look at it soon.

@airween airween mentioned this pull request

Apr 16, 2024

feat: Allow regular expressions in ctl:ruleRemoveTargetByX variable names II. #3121

Open

@airween

Copy link

Member

airween commented Apr 16, 2024

I'm closing this PR, I've already added it with PCRE2 macros in #3121. Thanks @vvidic.

@airween airween closed this

Apr 16, 2024

marcstern

This comment was marked as outdated.

marcstern

marcstern reviewed

Apr 17, 2024

View reviewed changes

Copy link

@marcstern marcstern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if((strlen(myvalue) == strlen(value)) && strncasecmp(myvalue,value,strlen(myvalue)) == 0)
Why not
if(strcasecmp(myvalue,value,strlen(myvalue)) == 0)

marcstern

marcstern reviewed

Apr 17, 2024

View reviewed changes

Copy link

@marcstern marcstern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else if (value == NULL && myvalue == NULL) {
 if (msr->txcfg->debuglog_level >= 9) {
 msr_log(msr, 9, "fetch_target_exception: Target %s will not be processed.", target);
 }
 match = 1;
} else if (value == NULL && myvalue != NULL) {
 if (msr->txcfg->debuglog_level >= 9) {
 msr_log(msr, 9, "fetch_target_exception: Target %s will not be processed.", target);
 }
 match = 1;
}

Simplify:

else {
 if (msr->txcfg->debuglog_level >= 9) {
 msr_log(msr, 9, "fetch_target_exception: Target %s will not be processed.", target);
 }
 match = 1;
}

Labels

2.x waiting for v3

7 participants

@vvidic @zimmerle @emphazer @fzipi @azurit @airween @marcstern

Allow regular expressions in ctl:ruleRemoveTargetByX variable names #911 #1683

Allow regular expressions in ctl:ruleRemoveTargetByX variable names #911 #1683

Uh oh!

Conversation

@vvidic vvidic commented Feb 23, 2018

Uh oh!

zimmerle commented Feb 28, 2018

Uh oh!

vvidic commented Feb 28, 2018

Uh oh!

zimmerle commented Feb 28, 2018

Uh oh!

emphazer commented Jan 4, 2019

Uh oh!

fzipi commented Jul 17, 2019

Uh oh!

vvidic commented Jul 17, 2019 via email

Uh oh!

fzipi commented Jul 21, 2019

Uh oh!

azurit commented Aug 27, 2019

Uh oh!

vvidic commented Aug 27, 2019 via email

Uh oh!

azurit commented Aug 27, 2019

Uh oh!

vvidic commented Aug 27, 2019 via email

Uh oh!

azurit commented Aug 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vvidic commented Aug 29, 2019 via email

Uh oh!

azurit commented Mar 12, 2020

Uh oh!

azurit commented Apr 16, 2024

Uh oh!

airween commented Apr 16, 2024

Uh oh!

airween commented Apr 16, 2024

Uh oh!

This comment was marked as outdated.

Uh oh!

@marcstern marcstern left a comment

Choose a reason for hiding this comment

Uh oh!

@marcstern marcstern left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

azurit commented Aug 29, 2019 •

edited

Loading