Merge pull request #3488 from umprayz/v2/cppcheck-initial

fix: cppcheck warnings in standalone/

Author: airween

.github/workflows/ci.yml

@@ -137,3 +137,41 @@ jobs:
         run: sudo make install
       - name: run regression tests
         run: make test-regression
+
+  cppcheck:
+    runs-on: [ubuntu-24.04]
+    container:
+      image: debian:sid
+    steps:
+      - name: Setup Dependencies
+        run: |
+              apt-get update -y -qq
+              apt-get install -y --no-install-recommends build-essential \
+              autoconf \
+              automake \
+              libtool \
+              pkg-config \
+              cppcheck \
+              apache2-dev \
+              libpcre2-dev \
+              libapr1-dev \
+              libaprutil1-dev \
+              libxml2-dev \
+              liblua5.3-dev \
+              libyajl-dev \
+              libfuzzy-dev \
+              ssdeep \
+              curl \
+              ca-certificates
+      - uses: actions/checkout@v4
+        with:
+          submodules: false
+          fetch-depth: 0
+      - name: configure
+        run: |
+          ./autogen.sh
+          ./configure --with-apxs=/usr/bin/apxs
+      - name: cppcheck
+        run: |
+          make check-static
+

Makefile.am

@@ -40,7 +40,27 @@ test-regression-nginx:
 
 
 cppcheck:
-	cppcheck . --enable=all --force 2>&1 | sed 's/^/warning: /g' 1>&2;
+	@cppcheck \
+	    -j `getconf _NPROCESSORS_ONLN 2>/dev/null || sysctl -n hw.ncpu || echo 1` \
+	    --enable=all \
+	    --force \
+	    --verbose \
+	    --library=gnu \
+	    --library=posix \
+	    --std=c++17 \
+		-I ./apache2 \
+	    -I /usr/include/libxml2 \
+		-I @APXS_INCLUDEDIR@ \
+	    -I @APR_INCLUDEDIR@ \
+	    -I @APU_INCLUDEDIR@ \
+	    --suppressions-list=./tests/cppcheck_suppressions.txt \
+	    --inline-suppr \
+	    --inconclusive \
+	    --template="warning: {file},{line},{severity},{id},{message}" \
+	    --error-exitcode=1 \
+	    standalone/
+
+check-static: cppcheck
 
 check-coding-style:
 	for i in `(find . -iname "*.c" ; find . -iname "*.h")`; \

autogen.sh

@@ -9,7 +9,7 @@ rm -rf autom4te.cache
 rm -f aclocal.m4
 case `uname` in Darwin*) glibtoolize --force --copy ;;
   *) libtoolize --force --copy ;; esac
-autoreconf --install
+autoreconf --install --force
 autoheader
 automake --add-missing --foreign --copy --force-missing
 autoconf --force

standalone/api.c

@@ -180,7 +180,6 @@ apr_status_t ap_http_in_filter(ap_filter_t *f, apr_bucket_brigade *bb_out,
     int is_eos = 0;
     apr_bucket_brigade *bb_in;
     apr_bucket *after;
-    apr_status_t rv;
 
     bb_in = modsecGetBodyBrigade(f->r);
 
@@ -191,7 +190,7 @@ apr_status_t ap_http_in_filter(ap_filter_t *f, apr_bucket_brigade *bb_out,
             APR_BRIGADE_INSERT_TAIL(bb_in, e);
         }
 
-        rv = apr_brigade_partition(bb_in, readbytes, &after);
+        apr_status_t rv = apr_brigade_partition(bb_in, readbytes, &after);
         if (rv != APR_SUCCESS && rv != APR_INCOMPLETE) {
             return rv;
         }
@@ -278,15 +277,15 @@ const char *modsecProcessConfig(directory_config *config, const char *file, cons
 
 		if(dir[li] != '/' && dir[li] != '\\')
 #ifdef	WIN32
-			file = apr_pstrcat(config->mp, dir, "\\", file, NULL);
+			file = apr_pstrcat(config->mp, dir, "\\", file, (char *)NULL);
 #else
-			file = apr_pstrcat(config->mp, dir, "/", file, NULL);
+			file = apr_pstrcat(config->mp, dir, "/", file, (char *)NULL);
 #endif
 		else
-			file = apr_pstrcat(config->mp, dir, file, NULL);
+			file = apr_pstrcat(config->mp, dir, file, (char *)NULL);
 	}
 	else if (APR_EBADPATH == status) {
-		return apr_pstrcat(config->mp, "Config file has a bad path, ", file, NULL);
+		return apr_pstrcat(config->mp, "Config file has a bad path, ", file, (char *)NULL);
 	}
 
 	apr_pool_create(&ptemp, config->mp);
@@ -403,7 +402,7 @@ request_rec *modsecNewRequest(conn_rec *connection, directory_config *config)
 
 static modsec_rec *retrieve_msr(request_rec *r) {
     modsec_rec *msr = NULL;
-    request_rec *rx = NULL;
+    const request_rec *rx = NULL;
 
     /* Look in the current request first. */
     msr = (modsec_rec *)apr_table_get(r->notes, NOTE_MSR);

standalone/api.h

@@ -58,7 +58,7 @@ void modsecInitProcess();
 
 conn_rec *modsecNewConnection();
 void modsecProcessConnection(conn_rec *c);
-int modsecFinishConnection(conn_rec *c);
+int modsecFinishConnection(conn_rec *c);
 
 request_rec *modsecNewRequest(conn_rec *connection, directory_config *config);
 
@@ -86,22 +86,40 @@ void modsecSetLogHook(void *obj, void (*hook)(void *obj, int level, char *str));
 
 static inline void 
 modsecSetBodyBrigade(request_rec *r, apr_bucket_brigade *b) {
+#ifdef __cplusplus
+    apr_table_setn(r->notes, NOTE_MSR_BRIGADE_REQUEST, reinterpret_cast<char *>(b)); //NOSONAR
+#else
     apr_table_setn(r->notes, NOTE_MSR_BRIGADE_REQUEST, (char *)b);
+#endif
 };
 
-static inline apr_bucket_brigade * 
-modsecGetBodyBrigade(request_rec *r) {
+static inline apr_bucket_brigade * modsecGetBodyBrigade(const request_rec *r) {
+#ifdef __cplusplus
+    return reinterpret_cast<apr_bucket_brigade *>(
+        const_cast<char *>(apr_table_get(r->notes, NOTE_MSR_BRIGADE_REQUEST))
+    );
+#else
     return (apr_bucket_brigade *)apr_table_get(r->notes, NOTE_MSR_BRIGADE_REQUEST);
+#endif
 };
 
 static inline void 
 modsecSetResponseBrigade(request_rec *r, apr_bucket_brigade *b) {
+#ifdef __cplusplus
+    apr_table_setn(r->notes, NOTE_MSR_BRIGADE_RESPONSE, reinterpret_cast<char *>(b)); //NOSONAR
+#else
     apr_table_setn(r->notes, NOTE_MSR_BRIGADE_RESPONSE, (char *)b);
+#endif
 };
 
-static inline apr_bucket_brigade * 
-modsecGetResponseBrigade(request_rec *r) {
+static inline apr_bucket_brigade * modsecGetResponseBrigade(const request_rec *r) {
+#ifdef __cplusplus
+    return reinterpret_cast<apr_bucket_brigade *>(
+        const_cast<char *>(apr_table_get(r->notes, NOTE_MSR_BRIGADE_RESPONSE))
+    );
+#else
     return (apr_bucket_brigade *)apr_table_get(r->notes, NOTE_MSR_BRIGADE_RESPONSE);
+#endif
 };
 
 void modsecSetReadBody(apr_status_t (*func)(request_rec *r, char *buf, unsigned int length, unsigned int *readcnt, int *is_eos));
@@ -121,7 +139,7 @@ const char *modsecIsServerSignatureAvailale(void);
 
 #ifdef VERSION_IIS
 void modsecStatusEngineCall(void);
-void modsecReportRemoteLoadedRules(void);
+void modsecReportRemoteLoadedRules(void);
 #endif
 
 #ifdef __cplusplus

standalone/buckets.c

@@ -57,7 +57,7 @@ AP_DECLARE(apr_status_t) ap_pass_brigade(ap_filter_t *next,
                                          apr_bucket_brigade *bb)
 {
     if (next) {
-        apr_bucket *e;
+        const apr_bucket *e;
         if ((e = APR_BRIGADE_LAST(bb)) && APR_BUCKET_IS_EOS(e) && next->r) {
             /* This is only safe because HTTP_HEADER filter is always in
              * the filter stack.   This ensures that there is ALWAYS a
@@ -89,7 +89,7 @@ AP_DECLARE(apr_status_t) ap_save_brigade(ap_filter_t *f,
                                          apr_bucket_brigade **b, apr_pool_t *p)
 {
     apr_bucket *e;
-    apr_status_t rv, srv = APR_SUCCESS;
+    apr_status_t srv = APR_SUCCESS;
 
     /* If have never stored any data in the filter, then we had better
      * create an empty bucket brigade so that we can concat.
@@ -98,11 +98,12 @@ AP_DECLARE(apr_status_t) ap_save_brigade(ap_filter_t *f,
         *saveto = apr_brigade_create(p, f->c->bucket_alloc);
     }
 
-    for (e = APR_BRIGADE_FIRST(*b);
-         e != APR_BRIGADE_SENTINEL(*b);
+    const apr_bucket_brigade *bb = *b;
+    for (e = APR_BRIGADE_FIRST(bb);
+         e != APR_BRIGADE_SENTINEL(bb);
          e = APR_BUCKET_NEXT(e))
     {
-        rv = apr_bucket_setaside(e, p);
+        apr_status_t rv = apr_bucket_setaside(e, p);
 
         /* If the bucket type does not implement setaside, then
          * (hopefully) morph it into a bucket type which does, and set