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

FPs with 941150 #1123

Closed
fgsch opened this issue Jun 8, 2018 · 12 comments
Closed

FPs with 941150 #1123

fgsch opened this issue Jun 8, 2018 · 12 comments
Labels
False Positive Stale issue This issue has been open 120 days with no activity.

Comments

@fgsch
Copy link
Contributor

fgsch commented Jun 8, 2018

Rule 941150 at PL2 is very prone to FPs, e.g. it will match if the QS containing src=foo.

We could move the ARGS* to a higher PL, or the rule altogether.

@dune73
Copy link
Contributor

dune73 commented Jun 9, 2018

How often do you get these? Depending on the application a lot, I presume. But how across multiple applications? src is a html attribute and I got the feeling this rule is quite effective vs. XSS.

@fgsch
Copy link
Contributor Author

fgsch commented Jun 9, 2018

I'm trying to get this information but anything can go in the query string really.
A quick search in httparchive shows it's not uncommon to see src and href, for example.

@dune73
Copy link
Contributor

dune73 commented Jun 9, 2018

Yes, absolutely. But then it's PL2 and writing a rule exclusion if you use the QS parameter src seems acceptable to me (not at PL1 though).

@fgsch
Copy link
Contributor Author

fgsch commented Jun 12, 2018

IMO rule exclusions should be the last resort. I'd prefer if we either move this to a higher PL or create a sibling at a higher PL to handle ARGS.

@dune73
Copy link
Contributor

dune73 commented Jun 12, 2018

That is a valid position, but not one that I share.

I think we need other opinions from other people to make a call.

@spartantri
Copy link
Contributor

Hi guys, this one in particular if it is a false positive to me it won't grant opening a hole on the CRS as I see this back firing with LFI/RFI/SSRF false negatives, that is better to catch at PL2 instead of dealing with PL3 debugging. In this case I would better deal with either exceptions, anomaly threshold levels, anomaly scores and the soon to come executing paranoia.
PL2 is supposed to be acceptable to take some false positives here and there (especially when it smells bad e.g. src=/e*/s*w)

@lifeforms
Copy link
Contributor

lifeforms commented Jun 12, 2018

Interestingly, I don't see a lot of false positives on this rule. In fact I have just one exclusion for this ruleId. On a cookie in fact. So for me this rule does seem okay in Paranoia Level 2.

@fgsch
Copy link
Contributor Author

fgsch commented Jun 12, 2018

Taking this upside down, how many are actually getting hits on this rule with the query string?

As I mentioned, having src and href in the query string is not uncommon.
While you might not be seeing it in your particular setup, it does not mean that FPs with this rule are rare.

@dune73
Copy link
Contributor

dune73 commented Jun 12, 2018

I've looked in a very big corpus of alerts triggered by burp. I saw the rule triggered 14 times. That is less than I expected.

However, @lifeforms is a hoster so you have various applications and my customers go with over 100 reverse proxies, mostly B2B applications. FPs are no big deal for this rule.

I think it really depends on a particular application and what it does with the query strings. Usually no problem, but some application will run into FPs systematically.

@fgsch
Copy link
Contributor Author

fgsch commented Jun 12, 2018

Fair enough. If you all reckon this rule is OK as it is then we can close this ticket.

@lifeforms
Copy link
Contributor

lifeforms commented Jun 12, 2018

I agree that src= and href= query parameters in URLs are not uncommon. For this reason we did remove checking Referer header from this rule, which I had quickly added earlier without much testing. See #547 and #585. So this rule is definitely a little bit controversial, although for me it seems to be working great.

URLs in themselves are not highly common inputs but we do regularly see them in "redirect to" or "get back to" parameters, and also in posted HTML content, so the FP risk is definitely there. But in my opinion it's an acceptable level for a rule that we put in PL2.

I do think it brings important protection to the table. Those types of "redirect to" parameters are often being reflected by an application, and XSS could play a role there.

Maybe we can make the pattern more specific in the future? We have to consider carefully browsers "autocorrecting" invalid HTML due to lax parsing however. So I see how the pattern might have been kept a bit minimal deliberately.

@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 Nov 19, 2019
@github-actions github-actions bot closed this as completed Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
False Positive Stale issue This issue has been open 120 days with no activity.
Projects
None yet
Development

No branches or pull requests

4 participants