wargio / naxsi

NAXSI is an open-source, high performance, low rules maintenance WAF for NGINX
GNU General Public License v3.0
305 stars 38 forks source link

Fix premature completion of checks for whitelisted internal rules #141

Closed lubomudr closed 5 months ago

lubomudr commented 5 months ago

Hi

Now, when internal rules are triggered, the check in most cases stops immediately and the remaining rules are not checked.

But internal rules can be whitelisted. In this case, the request is accepted even though the rule checking was interrupted.

In my opinion, you can stop checking only after checking that the rule is not in the white list.

Probably, after some rules have been triggered, it is impossible to continue checking, even if they are on the white list. For example, if the type “multipart/form-data” is specified, then you need to unambiguously stop checking if the format is violated. And even if the corresponding rule is in the white list, then reject the request. I can make a separate patch if necessary.

But, for example, in the absence of the “Content-type” header, according to the standard, it is quite possible to assume that “application/octet-stream” is meant. It might make sense to make a separate rule for this case, to distinguish between the absence of a header and an unknown content type

Or checking for incorrect UTF8 - such a value may well occur in binary data. And you can whitelist it and continue with the rest of the checks

Also added some checks for the success of memory allocation

wargio commented 5 months ago

Sorry for the late check, but i'm trying to update the documentation. I have this PR next into my list.

Thanks for the contribution :)

lubomudr commented 5 months ago

Maybe it would be more correct to check RAW_BODY not only when the Content-type is incorrect, but when any internal checks are triggered?

And in general, always check

wargio commented 5 months ago

Maybe it would be more correct to check RAW_BODY not only when the Content-type is incorrect, but when any internal checks are triggered?

The issue is that we do expect a content type because normal applications should always send it. those who don't are kinda weird requests.

Also the nx_int__uncommon_content_type which by reading the code is badly named, is also applied when a POST request does not match the following content type:

    "application/x-www-form-urlencoded"
    "multipart/form-data"
    "application/json"
    "application/vnd.api+json"
    "application/csp-report"

I think this kinda make sense because i would expect these to be the most used ones in post requests. also when uploading files.

And in general, always check

On this, i do agree.