Skip to content
This repository has been archived by the owner on May 14, 2020. It is now read-only.

Rules matched at REQUEST_BODY instead of ARGS #1518

Closed
azurit opened this issue Aug 24, 2019 · 19 comments
Closed

Rules matched at REQUEST_BODY instead of ARGS #1518

azurit opened this issue Aug 24, 2019 · 19 comments
Assignees
Labels
PR available this issue is referenced by an active pull request

Comments

@azurit
Copy link
Contributor

azurit commented Aug 24, 2019

Type of Issue

Bug

Description

Rules 930100 and 930110 are getting matched at whole REQUEST_BODY instead of ARGS:redirect . I sent the full log to folini@netnea.com.

Your Environment

  • CRS version (e.g. v3.0.2): 3.1.1
  • ModSecurity version (e.g. 2.9.2): 2.9.3
  • Web Server and version (e.g. apache 2.4.27): 2.4.25
  • Operating System and version: Debian Stretch

Confirmation

[x] I have removed any personal data (email addresses, IP addresses,
passwords, domain names) from any logs posted.

@github-actions
Copy link

This issue has been open 120 days with no activity. Remove the stale label or comment, or this will be closed in 14 days

@github-actions github-actions bot added the Stale issue This issue has been open 120 days with no activity. label Dec 23, 2019
@azurit
Copy link
Contributor Author

azurit commented Dec 23, 2019

We are still having problems with this (maybe also with other rules).

@theMiddleBlue
Copy link
Contributor

Hi @azurit

could you please provide the full blocked request log here too? Anyway, If your request is blocked by both rules because of something inside ARGS:redirect, a possible solution could be to remove ARGS:redirect evaluation from 930100 and 930110.

@azurit
Copy link
Contributor Author

azurit commented Dec 23, 2019

I don't have that log anymore (but it's quite common issue, maybe i will get another one soon).

The problem which i see here is when i write a rule to exclude, for example, ARGS:redirect, the modsecurity is, sometimes (NOT always of course), matching the SAME problem second time for, this time for whole REQUEST_BODY. TO bypass this, i need to exclude whole REQUEST_BODY, which means that i'm excluding also other arguments (which i don't want, i may need to keep other arguments secured).

I don't know how to explain this problem better. It's like, in regexp, using .* instead of .*? ('smallest' match).

@theMiddleBlue
Copy link
Contributor

Oh okay, I got it now. It should be solved replacing REQUEST_BODY with ARGS so you can write your exclusion rule. Is it?

@azurit
Copy link
Contributor Author

azurit commented Dec 23, 2019

No, as i said, i alredy have exclusion rule for ARGS but modsecurity is triggering the same thing again but this time on REQUEST_BODY (so i'm forced to rewrite the rule to exclude whole REQUEST_BODY).

@fgsch fgsch removed the Stale issue This issue has been open 120 days with no activity. label Dec 23, 2019
@theMiddleBlue
Copy link
Contributor

Yes, I mean we should replace REQUEST_BODY in both rules (930100 and 930110) with ARGS.

@fgsch
Copy link
Contributor

fgsch commented Dec 23, 2019

@dune73 do you have the logs @azurit sent? Is there any information that can be shared from them?

@dune73
Copy link
Contributor

dune73 commented Dec 25, 2019

I'm sure I do. I remember receiving it. But I can't seem to find it anymore.

Who was the sender @azurit? The email address, I mean, so I have a keyword to search my archive.

@azurit
Copy link
Contributor Author

azurit commented Dec 25, 2019

@dune73 I sent you another e-mail with list of address (don't remember which one i used). Thank you.

@dune73
Copy link
Contributor

dune73 commented Jan 3, 2020

Dug out the message / attachment in question successfully. Forwarded to @fgsch and @azurit.

@fgsch
Copy link
Contributor

fgsch commented Jan 6, 2020

@dune73 thanks. I had a quick look and I think what @theMiddleBlue wrote makes sense. We should replace REQUEST_BODY with ARGS so a rule to exclude specific arguments can be written.

@fgsch
Copy link
Contributor

fgsch commented Jan 6, 2020

@theMiddleBlue Do you want to open a PR or should I take a look?

@theMiddleBlue
Copy link
Contributor

theMiddleBlue commented Jan 6, 2020

Sure! Thanks. I'll open a PR

@fgsch fgsch added the PR available this issue is referenced by an active pull request label Jan 8, 2020
@theMiddleBlue
Copy link
Contributor

should be fixed by #1659

@Taiki-San
Copy link
Contributor

Hey! Sorry for arriving late to the party but this change introduced many false positives to the rule.
I'm seeing many of our users with patterns similar to http://url?query=../img/social_icons.png.
Although that could be construed as a LFI vulnerability, this case is a false positive.

We had already forked the rule to run on ARGS at a lower confidence level but with this change, the main rule become dangerous.

@azurit
Copy link
Contributor Author

azurit commented May 12, 2020

@Taiki-San Hi, how this change could introduced new FPs? Before, it was matching the whole REQUEST_BODY (including ARGS) and now it is matching only ARGS.

@Taiki-San
Copy link
Contributor

@azurit REQUEST_BODY is a subset of ARGS, at least according to the reference.
REQUEST_BODY only contains the POST payload, while ARGS also include the query string. Our false positives are on parameters coming from query strings.

@azurit
Copy link
Contributor Author

azurit commented May 12, 2020

Ok, i see. In this case, it should be set to ARGS_POST instead of ARGS to keep backward compatibility.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR available this issue is referenced by an active pull request
Projects
None yet
Development

No branches or pull requests

5 participants