libinjection icon indicating copy to clipboard operation
libinjection copied to clipboard

Rework Pullrequest #122, avoid xss false positives starting with 'on.*'

Open blappm opened this issue 6 years ago • 8 comments

It is safer to use a list of event handlers than just matching strings > 5 chars

blappm avatar Nov 16 '18 14:11 blappm

As a CRS maintainer, I agree that a fix for this problem would be very interesting. Our users regularly turn up false positives due to generic onfoo= matching. Some examples:

https://github.com/SpiderLabs/owasp-modsecurity-crs/issues/820 https://github.com/SpiderLabs/owasp-modsecurity-crs/issues/967

A discrete blacklist would solve this problem, although it may require more regular maintenance as new event handlers are added.

lifeforms avatar Nov 16 '18 15:11 lifeforms

Looks like there were some eventhandlers missing. Adding them now.

blappm avatar Nov 16 '18 15:11 blappm

build passed 🎉

lifeforms avatar Nov 16 '18 15:11 lifeforms

Well :-) It's now sorted alphabetically. This makes it easier to add new event handlers.

blappm avatar Nov 16 '18 16:11 blappm

We are now successfully using this patch in production. While we were seeing 20-30 FP per day before, the rate has now dropped to 1-2 per day.

One of the worst FP caused by this was 'online'.

blappm avatar Nov 19 '18 15:11 blappm

Is there anything holding this PR? It'd be great if it's merged.

fgsch avatar Jan 03 '19 19:01 fgsch

@client9 is this project abandoned?

frankyhun avatar Apr 29 '20 14:04 frankyhun

@client9 is this project abandoned?

You may want to look here: libinjection/libinjection#7 we are giving o followup on that discussion there.

zimmerle avatar Jan 14 '21 17:01 zimmerle