Skip to content

Macro expansion inside regex does not work in ModSecurity V3 #1528

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

Closed
asterite3 opened this issue Aug 3, 2017 · 1 comment
Closed

Macro expansion inside regex does not work in ModSecurity V3 #1528

asterite3 opened this issue Aug 3, 2017 · 1 comment
Assignees

Comments

@asterite3
Copy link

Documentation on macro expansion says (https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual#macro-expansion): You can use macro expansion for operators that are "compiled" such @rx, etc. however you will have some impact in efficiency..
In V3, macro expansion in regexes is not performed and operator "@rx ^%{var}$" will match data against literal ^%{var}$.
The following code can be used for demonstration:

#include "modsecurity/modsecurity.h"
#include "modsecurity/rules.h"

static void logCb(void *data, const void *ruleMessagev) {
    std::cout << (char *) ruleMessagev << std::endl;
}

const char * req_uri = "/test?param=attack";

int main() {
    modsecurity::ModSecurity *modsec = new modsecurity::ModSecurity();
    modsecurity::Rules *rules = new modsecurity::Rules();
    modsecurity::Transaction * modsecTransaction;

    modsec->setServerLogCb(logCb, modsecurity::TextLogProperty);

    rules->loadFromUri("test.conf");

    modsecTransaction = new modsecurity::Transaction(modsec, rules, NULL);
    modsecTransaction->processURI(req_uri, "GET", "1.1");
    modsecTransaction->processRequestHeaders();
    modsecTransaction->processRequestBody();

    delete modsecTransaction;
    delete rules;
    delete modsec;
    return 0;
}

With following test.conf:

SecAction "id:1, nolog, setvar:tx.bad_value=attack"

SecRule ARGS:param "@rx ^%{tx.bad_value}$" id:2

It won't produce expected alert because modsec will match param against literal ^%{tx.bad_value}$ instead of ^%attack$.

This breaks OWASP CRS rule 920420 which uses macro expansion of variable tx.allowed_request_content_type (this part of the rule will fire for any request because negated operator is used):

SecRule TX:0 "!^%{tx.allowed_request_content_type}$"
asterite3 added a commit to seclab-msu/ModSecurity that referenced this issue Aug 19, 2017
try to use macro expansion on @rx argument before matching.
If after expansion argument changed, make new Regex from
the macro-expanded argument and use that for matching.
Fixes owasp-modsecurity#1528
zimmerle pushed a commit that referenced this issue Oct 6, 2017
try to use macro expansion on @rx argument before matching.
If after expansion argument changed, make new Regex from
the macro-expanded argument and use that for matching.
Fixes #1528
@zimmerle zimmerle self-assigned this Oct 6, 2017
@zimmerle
Copy link
Contributor

zimmerle commented Oct 6, 2017

Fixed at fa7973a

@zimmerle zimmerle closed this as completed Oct 6, 2017
zimmerle pushed a commit that referenced this issue Jan 3, 2018
On #1528 was added the support for macro expansion on @rx operator.
The performance improvement suggested on the pull request was not
thread safe, therefore removed. This patch adds a performance
improvement on top of #1528. The benchmarks points to 10x faster
results on OWASP CRS.
zimmerle pushed a commit that referenced this issue Jan 3, 2018
On #1528 was added the support for macro expansion on @rx operator.
The performance improvement suggested on the pull request was not
thread safe, therefore removed. This patch adds a performance
improvement on top of #1528. The benchmarks points to 10x faster
results on OWASP CRS.
zimmerle pushed a commit that referenced this issue Feb 20, 2018
On #1528 was added the support for macro expansion on @rx operator.
The performance improvement suggested on the pull request was not
thread safe, therefore removed. This patch adds a performance
improvement on top of #1528. The benchmarks points to 10x faster
results on OWASP CRS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants