coreruleset / nextcloud-rule-exclusions-plugin

Rule exclusion plugin for Nextcloud
Apache License 2.0
11 stars 7 forks source link

fix: text editor in public share and token related FPs #59

Closed EsadCetiner closed 6 months ago

EsadCetiner commented 7 months ago

This PR mostly fixes issues with session tokens that are only an issue at PL-2 and above, but there is a fix for using the markdown test editor in public shares. The tests for some of these rules were very large(This PR would've been above 700 lines, but now it's about 300), so I refactored them by using a | to test against multiple URIs with the same payload, but for whatever reason the nginx regression tests are failing while the apache tests are passing.

theseion commented 7 months ago

I'll check it out tomorrow.

theseion commented 7 months ago

This will create an illegal request:

uri: |
  uri1
  uri2

This is the same issue you had with the regular expressions: the vertical bar | in YAML means "verbatim string", not "list". The request generated from the above would look like this:

PUT /apps/text/session/create
/apps/text/public/session/create
 HTTP/1.1
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Connection: close
Content-Length: 0
Host: localhost
User-Agent: OWASP CRS

As you can see, the URIs are used as the request URI verbatim, including line breaks.

There is currently no way to run the same test against multiple URIs (would be great though). You could use multiple stages per test but that only makes a small difference and isn't what stages are designed for. For now, you don't have any other option than to duplicate tests for different URIs.

theseion commented 7 months ago

I've created a feature request in go-ftw: https://github.com/coreruleset/go-ftw/issues/256

EsadCetiner commented 7 months ago

@theseion Thanks for the explanation, guess we'll have to live with it for now until go-ftw adds support for what I'm trying to do.