owasp-modsecurity / ModSecurity

ModSecurity is an open source, cross platform web application firewall (WAF) engine for Apache, IIS and Nginx. It has a robust event-based programming language which provides protection from a range of attacks against web applications and allows for HTTP traffic monitoring, logging and real-time analysis.
https://www.modsecurity.org
Apache License 2.0
7.67k stars 1.54k forks source link

NULL pointer checks & compiler warnings #3105

Open marcstern opened 2 months ago

marcstern commented 2 months ago

I'm rewriting my PR for NULL pointer checks. In most places, it's as basic as adding "if (ptr != NULL)" but I tried to optimize it a bit. There are some cases where a pointer cannot be NULL; it just doesn't make sense. Such an example is the "msc" variable used at a lot of place. It's initialized to non-NULL and checked, so it cannot be NULL (except in msc_tee.c, but that's another discussion). So, there's no need to check it in every single function. However, we'll get (depending on the compiler) warnings about null pointer dereference. Also, when doing a code review, it's not clear if it can never be null or if the developers forgot to check it. I found an almost neat solution: add "assert(msc != NULL)" in every function. It clarifies the code and suppresses the compiler warning ... at least in debug mode because assert() is defined to nothing in NDEBUG mode so the check is not executed - good from a performance point of view) and the compiler doesn't see that we check for nulliness. Does anybody see a variation that would avoid the compiler warning without adding a real check at run-time?

gberkes commented 2 months ago

FYI

https://github.com/owasp-modsecurity/ModSecurity/pull/3104