fix: cppcheck warnings in standalone/ #3487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

umprayz wants to merge 25 commits into owasp-modsecurity:v2/master from umprayz:v2/cppcheck

.github/workflows/ci.yml

-Original file line number
+Diff line change
@@ Expand Up / @@ -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

100644 → 100755

-Original file line number
+Diff line change
@@ Expand Up / @@ -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")`; \
@@ Expand Down @@

autogen.sh

-Original file line number
+Diff line change
@@ Expand Up / @@ -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
@@ Expand Down @@

standalone/api.c

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -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

-Original file line number
+Diff line change
@@ Expand Up / @@ -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);
@@ Expand Down Expand Up @@
     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));
@@ Expand All / @@ -121,7 +139,7 @@ const char *modsecIsServerSignatureAvailale(void); @@
     #ifdef VERSION_IIS
     void modsecStatusEngineCall(void);
-    void modsecReportRemoteLoadedRules(void);
+    void modsecReportRemoteLoadedRules(void);
     #endif
     #ifdef __cplusplus
@@ Expand Down @@

standalone/buckets.c

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: cppcheck warnings in standalone/ #3487

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!