LFI path traversal rules using REQUEST_BODY cannot be excluded

[lifeforms]

I'm tuning for WordPress FP right now, and I want to exclude some fields. For instance, I'd like to allow people to use the ../ sequence (LFI, rule 930110) in a password field.

However, it's currently impossible for me to exclude this LFI check from a parameter. Normally I would do ctl:ruleRemoveTargetByTag=CRS;ARGS:passwd to exclude the parameter passwd from all our rules.

But rule 930110 is still firing on ../ in a password string. The reason is that rule 930110 is not checking ARGS, but instead it is checking the following variables instead:

REQUEST_URI_RAW|REQUEST_BODY|REQUEST_HEADERS|XML:/*

This makes it impossible to selectively exclude this rule for a single query parameter, unless I want to remove the whole REQUEST_BODY target which I'd prefer not to do.

The raw uri/body check is rather rare in the CRS; rule 930100 also has it. I think I can see why the body is checked -- an unencoded post body (stdin) is often used for code injection, plus perhaps it could have a performance advantage over scanning the args. I see that the commit, 0a035cc, was made in 2014 by Ryan. Do you recall why this approach was taken @rcbarnett? (If you have time to look at it!)

I think a body payload should turn up in ARGS_NAMES as well, so we could probably switch the rule's variables over without losing detection. But I'd want to add a lot of tests for it. Realistically it might probably be too late for 3.0, although I could see more people hitting this problem.

[lifeforms]

As predicted I've got some FP complaints in CRS3 about this one, and it's impossible to write ARGS exclusions for it, except for disabling the whole rule :(

[dune73]

No feedback from @rbarnett on some other channel?

[rbarnett]

@dune73 wrong rbarnett 😃

[lifeforms]

Another false positive popped up in #783 which underscores that we really should check ARGS and not the whole REQUEST_BODY.

Edit: if there are good reasons for scanning REQUEST_BODY, which may very well be so (and if we lost the institutional knowledge #783 might give us a hint) I think we could move the current rule to paranoia level 2, and introduce a less broad one at level 1.

[lifeforms]

Another person was bitten by this problem in #1264.

One big problem is the rule running on REQUEST_BODY instead of ARGS.

Another problem with the rule is that ../ is a too eager pattern. This sequence happens too often in the middle of benign content. In-house I use a different rule for this, that triggers only on:

• ^../ start of string
• "../ start of a json-encoded string
• /../ possibly part of a path
• the three above with t:base64DecodeExt transformation

(That's for unix path separator / only since we're not running Windows, but we could add \ as well.)

I could do more analysis, but when I last looked at it a long time ago, these appeared to catch all my dir traversal attempts in prod. Are there more preceding characters that set up a dangerous context for ../?

Then we have the final question, the amazingly complex and rich regexp that's in our rule currently. Does it have any magic which I'm missing? Probably... in which case we could keep the current regexp in PL2 as a safety measure.

So in summary I would suggest:

• New rule(s) for PL1 with small, easy regexp that ignores likely non-vulnerable contexts
• Move our old REQUEST_BODY LFI ruleId into Paranoia Level 2

[welljsjs]

Sorry for interfering, but I recently ran into the same issue.

I was just curious whether there's any update on this.

What's the reason for not using ARGS instead of REQUEST_BODY?

[lifeforms]

I wish I knew, BUT my patience with this rule is getting thin. Let's fix it in the next release.

[mhoran]

I just hit this after enabling modsecurity and the CRS for my WordPress site. I had a draft post that had a sentence ending in "...". Of course, the thing that comes after "..." is a newline, or "\n" -- which triggers rule 930110 and 942440. The WordPress exclusions don't do anything to handle this, so I couldn't save my post. I deleted it and recreated without the "...", but fortunately I had access to the modsecurity audit log otherwise I would have had no idea what was happening.

[dune73]

It's an annoying problem and we should take care of it. But with so many other things going on, it did not get priority so far. Sorry.

[dune73]

@emphazer volunteered to take on this issue as he has been affected before. This is likely to take until March 2020 though.

Meeting minutes: #1671 (comment)