_Issue originally created by user dune73 on date 2018-02-23 05:44:01.
Link to original issue: https://github.com/SpiderLabs/owasp-modsecurity-crs/issues/1022._
There is a false negative in the way we ignore onXXX events in the URI.
curl "http://localhost/index.html/x%22%3E%3CsvG%20onLoad=prompt(9)%3E"
If the same URI appears in the Referer, it triggers 941101, 941120 and 941160. Adding the URI to be covered by these rules, won't change a thing, so it's also an encoding problem.
_User csanders-git commented on date 2018-02-27 13:19:53:_
So is the suggestion to add the URI to one of these rules
_User dune73 commented on date 2018-02-27 15:10:11:_
Not sure yet. But my customer just confirmed he wants me to come up with a solution here. So I'll dig into this and solve it.
_User csanders-git commented on date 2018-02-27 17:09:05:_
I remember we used to have something for URL rewriting
_User spartantri commented on date 2018-02-28 15:26:47:_
This is weird, the web server should reply with a 404 due to non existing resource as index.html is a directory and the x"> I'm curious, what web server do they use? May they have a bad url rewriting pattern in place that allows this funny things to be accepted by turning that into something else? do you have a sample response?
_User dune73 commented on date 2018-02-28 15:40:04:_
;)
I rewrote the filename part of the URI to camouflage the real filename. Obviously I omitted the hostname, the slash and any real thought at a reasonable filename.
However, I do think enterprise CMS systems are more sophisticated than what you think. To the client, this index.html looks like a static page, but for the CMS it's dynamic ressource and /x%22... are all parameters / PATH_INFO for the request.
_User spartantri commented on date 2018-03-01 08:56:53:_
There you go, I had exactly that in mind, this is not the web server it is the CMS doing bad rewrites to turn /x into ?x after modsec inspection without proper validation plus a nice non-escaped content reflection or store after that, don't you love developers, please tell me this is a php app to have a poker of nice little stuff in hand.
To me modsec REQUEST_FILENAME or REQUEST_URI vars could be added to the rules that check for ARGS and do not include on their inspection valid path and filename characters like / \ - _ . on PL1 to have the least of false positives but expect to have lot of issues and maybe a complete disaster on higher PL especially with those \W we have around so it may be good for PL1 and maybe PL2 but bad a complete no go for PL3 and PL4.
I will be happy to see more inspections on headers and request line that what he have now, we are ignoring lots of fun stuff.
_User fgsch commented on date 2019-02-12 10:30:30:_
Another example (redacted): /something/other%3C%2Fscript%3E%3Cscript%3Ealert(1)%3C%2Fscript%3E/
Is there any update on this dune73 ?
_User fgsch commented on date 2019-02-12 11:17:17:_
Can we start by adding the path to 941110? Or anyone thinks this will cause many FPs?
_User dune73 commented on date 2019-02-12 16:11:53:_
No progress. Sorry. I know it's on my todo list, but my life outside CRS is eating me alive these days. If anybody wants to take over, be my guest.
I think adding the path would make sense. lifeforms: What do you think?
_User theMiddleBlue commented on date 2019-02-12 16:29:27:_
agree with fgsch. IMHO all XSS rules should look into REQUEST_FILENAME too
_User fgsch commented on date 2019-02-12 16:46:28:_
I will submit a PR later today or tomorrow for 941110. I haven't had a chance to look into the other vectors in detail but I expect FPs at least until https://github.com/client9/libinjection/pull/143 is merged, which seems to have gone awfully silent.
_User github-actions[bot] commented on date 2019-11-20 00:03:46:_
This issue has been open 120 days with no activity. Remove the stale label or comment, or this will be closed in 14 days
Most helpful comment
_User fgsch commented on date 2019-02-12 16:46:28:_
I will submit a PR later today or tomorrow for 941110. I haven't had a chance to look into the other vectors in detail but I expect FPs at least until https://github.com/client9/libinjection/pull/143 is merged, which seems to have gone awfully silent.