Build error related to APR in config.c

[Marcool04]

Describe the bug

Building fails with these two errors related to APR:

config.c: In function ‘process_command_config’:                                                                        08:40:49 [85/3634]
config.c:1107:60: error: passing argument 1 of ‘apr_filepath_root’ from incompatible pointer type [-Wincompatible-pointer-types]         
 1107 |                                 status = apr_filepath_root(&rootpath, &incpath, APR_FILEPATH_TRUENAME | APR_FILEPATH_NATIVE, ptem
p);                                                                                                                                      
      |                                                            ^~~~~~~~~                                                             
      |                                                            |                                                                     
      |                                                            char **                                                               
In file included from /usr/include/apr-1/apr_file_io.h:29,                                                                               
                 from /usr/include/apr-1/apr_network_io.h:26,                                                                            
                 from /usr/include/apr-1/apr_buckets.h:29,                                                                               
                 from /usr/include/httpd/util_filter.h:26,                                                                               
                 from /usr/include/httpd/http_core.h:32,                                                                                 
                 from config.c:18:                                                                                                       
/usr/include/apr-1/apr_file_info.h:336:58: note: expected ‘const char **’ but argument is of type ‘char **’                              
  336 | APR_DECLARE(apr_status_t) apr_filepath_root(const char **rootpath,                                                               
      |                                             ~~~~~~~~~~~~~^~~~~~~~                                                                
config.c:1107:71: error: passing argument 2 of ‘apr_filepath_root’ from incompatible pointer type [-Wincompatible-pointer-types]         
 1107 |                                 status = apr_filepath_root(&rootpath, &incpath, APR_FILEPATH_TRUENAME | APR_FILEPATH_NATIVE, ptem
p);                                                                                                                                      
      |                                                                       ^~~~~~~~                                                   
      |                                                                       |                                                          
      |                                                                       char **                                                    
/usr/include/apr-1/apr_file_info.h:337:58: note: expected ‘const char **’ but argument is of type ‘char **’                              
  337 |                                             const char **filepath,                                                               
      |                                             ~~~~~~~~~~~~~^~~~~~~~

Logs and dumps

Full build log is here: https://termbin.com/4pis (too big for a github comment)

To Reproduce

The build script is this one: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=libmodsecurity2 equivalent to getting the latest tagged v2 release (v2.9.7 and doing:

  ./autogen.sh
  ./configure \
    --prefix=/usr \
    --enable-standalone-module \
    --enable-htaccess-config
  echo "Fixing libtool for hardcoded_into_libs"
  sed -ri 's|(hardcode_into_libs)=.*|\1=no|' libtool
  echo "Fixing apache2/msc_lua.c to accept lua 5.4"
  sed -i 's#LUA_VERSION_NUM == 502 || LUA_VERSION_NUM == 503#LUA_VERSION_NUM == 502 || LUA_VERSION_NUM == 503 || LUA_VERSION_NUM == 504#' "apache2/msc_lua.c"
  sed -ri 's/We are only tested under Lua 5.0, 5.1, 5.2, or 5.3./We are only tested under Lua 5.0, 5.1, 5.2, or 5.3 (and faking 5.4)./' "apache2/msc_lua.c"
  make

Expected behavior

Successful build.

Server (please complete the following information):

• ModSecurity version (and connector): modsecurity v2.9.7
• WebServer: N/A
• OS (and distro): Arch linux

Rule Set (please complete the following information):
N/A

Additional context

Since the error is coming from the file /usr/include/apr-1/apr_file_io.h I checked that out, it is owned by package apr, so I checked if any recent changes had occurred there but we have:

$ pacman -F /usr/include/apr-1/apr_file_io.h                                                               
usr/include/apr-1/apr_file_io.h is owned by extra/apr 1.7.4-1 

$ pacman -Qi apr
Name            : apr
Version         : 1.7.4-1
Description     : The Apache Portable Runtime
[snip]
Build Date      : Wed 25 Oct 2023 08:24:16 PM CEST
Install Date    : Sun 29 Oct 2023 09:43:43 AM CET

so this is old stuff...

[airween]

Hi @Marcool04,

thanks for this detailed report.

Even tough this is a bug, but may be some CFLAG/C restriction blocks the successful build.

I also get this message but not as an error, but "just" a warning:

config.c: In function ‘process_command_config’:
config.c:1107:32: warning: passing argument 1 of ‘apr_filepath_root’ from incompatible pointer type [-Wincompatible-pointer-types]
 1107 |     status = apr_filepath_root(&rootpath, &incpath, APR_FILEPATH_TRUENAME | APR_FILEPATH_NATIVE, ptemp);
      |                                ^~~~~~~~~
      |                                |
      |                                char **
In file included from /usr/include/apr-1.0/apr_file_io.h:29,
                 from /usr/include/apr-1.0/apr_network_io.h:26,
                 from /usr/include/apr-1.0/apr_buckets.h:29,
                 from /usr/include/apache2/util_filter.h:26,
                 from /usr/include/apache2/http_core.h:32,
                 from config.c:18:
/usr/include/apr-1.0/apr_file_info.h:336:58: note: expected ‘const char **’ but argument is of type ‘char **’
  336 | APR_DECLARE(apr_status_t) apr_filepath_root(const char **rootpath,
      |                                             ~~~~~~~~~~~~~^~~~~~~~
config.c:1107:43: warning: passing argument 2 of ‘apr_filepath_root’ from incompatible pointer type [-Wincompatible-pointer-types]
 1107 |     status = apr_filepath_root(&rootpath, &incpath, APR_FILEPATH_TRUENAME | APR_FILEPATH_NATIVE, ptemp);
      |                                           ^~~~~~~~
      |                                           |
      |                                           char **
In file included from /usr/include/apr-1.0/apr_file_io.h:29,
                 from /usr/include/apr-1.0/apr_network_io.h:26,
                 from /usr/include/apr-1.0/apr_buckets.h:29,
                 from /usr/include/apache2/util_filter.h:26,
                 from /usr/include/apache2/http_core.h:32,
                 from config.c:18:
/usr/include/apr-1.0/apr_file_info.h:337:58: note: expected ‘const char **’ but argument is of type ‘char **’
  337 |                                             const char **filepath,
      |                                             ~~~~~~~~~~~~~^~~~~~~~

Unfortunately based on the given data (including your excellent build log) I wasn't able to reproduce this issue. I tried with similar settings (eg. passed -Werror=format-security to CFLAGS), but no luck.

Could you review your settings?

[Marcool04]

Thanks for looking into this @airween , glad the report was clear.
I have this when I run env just before make:

SHELL=/bin/bash                                                                                                                          
BUILDTOOL=devtools                                                                                                                       
SUDO_GID=0                                                                                                                               
PYTHONHASHSEED=0                                                                                                                         
TEXTDOMAINDIR=/usr/share/locale                                                                                                          
SUDO_COMMAND=/bin/bash -c bash -c cd\ /startdir;\ makepkg\ "$@" -bash --syncdeps --noconfirm --log --holdver --skipinteg --install       
SUDO_USER=root
PWD=/build/libmodsecurity2/src/modsecurity-2.9.7                                                                     23:21:55 [793/10743]
SOURCE_DATE_EPOCH=1718659294                                                                                                             
LOGNAME=builduser                                                                                                                        
CXXFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security   
      -fstack-clash-protection -fcf-protection         -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wp,-D_GLIBCXX_ASSERTIONS -g 
-ffile-prefix-map=/build/libmodsecurity2/src=/usr/src/debug/libmodsecurity2 -flto=auto                                                   
DEBUG_RUSTFLAGS=-C debuginfo=2 --remap-path-prefix=/build/libmodsecurity2/src=/usr/src/debug/libmodsecurity2                             
COMMAND_MODE=legacy                                                                                                                      
LDFLAGS=-Wl,-O1 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now          -Wl,-z,pack-relative-relocs -flto=auto                
HOME=/build                                                                                                                              
LANG=C.UTF-8                                                                                                                             
RUSTFLAGS=-Cforce-frame-pointers=yes -C debuginfo=2 --remap-path-prefix=/build/libmodsecurity2/src=/usr/src/debug/libmodsecurity2        
MAKEFLAGS=-j10                                                                                                                           
TERM=tmux-256color                                                                                                                       
USER=builduser                                                                                                                           
SHLVL=1                                                                                                                                  
DEBUGINFOD_URLS=https://debuginfod.archlinux.org                                                                                         
CHOST=x86_64-pc-linux-gnu                                                                                                                
PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl                                  
CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security     
    -fstack-clash-protection -fcf-protection         -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -ffile-prefix-map=/build/lib
modsecurity2/src=/usr/src/debug/libmodsecurity2 -flto=auto
SUDO_UID=0
MAIL=/var/mail/builduser
BUILDTOOLVER=1:1.2.0-1-any
OLDPWD=/build/libmodsecurity2/src
TEXTDOMAIN=pacman-scripts
_=/usr/bin/env

nothing in there stands out to me... How about you?

I also tried export CFLAGS="${CFLAGS} -Wno-error=incompatible-pointer-types just before make and that made no difference...

I'm a bit stuck for ideas here. BTW this is being run via an Arch linux build script that runs the compilation in a clean, up to data chroot. Usually in this context the Arch default build flags for C are pretty mundane.

[marcstern]

'rootpath' must indeed be 'const'.
Note that 'rootpath' is used later for a totally different treatment - see if (APR_ERELATIVE == status). This should use another non-const variable.

[Marcool04]

One possible explanation here for this being considered an error on my end and a warning elsewhere could be the fact that Arch uses quite bleeding-edge versions of software?

$ pacman -Si gcc
Repository      : core
Name            : gcc
Version         : 14.1.1+r58+gfc9fb69ad62-1
Description     : The GNU Compiler Collection - C and C++ frontends
Architecture    : x86_64
[snip]
Build Date      : Wed 22 May 2024 02:09:02 PM CEST

A bit more digging reveals this is indeed the case. From https://gcc.gnu.org/gcc-14/changes.html :

C: Certain warnings about are now errors, see Porting to GCC 14 for details.

And there we have:

Type checking on pointer types (-Werror=incompatible-pointer-types)
GCC no longer allows implicitly casting all pointer types to all other pointer types. This behavior is now restricted to the void * type and its qualified variations.

To fix compilation errors resulting from that, you can add the appropriate casts, and maybe consider using void * in more places (particularly for old programs that predate the introduction of void * into the C language).

Programs that do not carefully track pointer types are likely to contain aliasing violations, so consider building with -fno-strict-aliasing. (Whether casts are written manually or performed by GCC automatically does not make a difference in terms of strict aliasing violations.)

A frequent source of incompatible function pointer types involves callback functions that have more specific argument types (or less specific return types) than the function pointer they are assigned to. For example, old code which attempts to sort an array of strings might look like this:

#include <stddef.h>
#include <stdlib.h>
#include <string.h>

int
compare (char **a, char **b)
{
return strcmp (*a, *b);
}

void
sort (char **array, size_t length)
{
qsort (array, length, sizeof (*array), compare);
}

To address this, the callback function should be defined with the correct type, and the arguments should be cast as appropriate before they are used (as calling a function through a function pointer of an incorrect type is undefined):

int
compare (const void *a1, const void *b1)
{
char *const *a = a1;
char *const *b = b1;
return strcmp (*a, *b);
}

A similar issue can arise with object pointer types. Consider a function that is declared with an argument of type void **, and you want to pass the address of a variable of type int *:

extern int *pointer;
extern void operate (int command, void **);

operate (0, &pointer);

In these cases, it may be appropriate to make a copy of the pointer with the correct void * type:

extern int *pointer;
extern void operate (int command, void **);

void *pointer1 = pointer;
operate (0, &pointer1);
pointer = pointer1;

As mentioned initially, adding the cast here would not eliminate any strict aliasing violation in the implementation of the operate function. Of course in general, introducing such additional copies may alter program behavior.

Some programming styles rely on implicit casts between related object pointer types to implement C++-style struct inheritance. It may be possible to avoid writing manual casts with the -fplan9-extensions option and unnamed initial struct fields for the base type in derived structs.

Some programs use a concrete function pointer type such as void (*) (void) as a generic function pointer type (similar to void * for object pointer types), and rely on implicit casts to and from this type. The reason for that is that C does not offer a generic function pointer type, and standard C disallows casts between function pointer types and object pointer types. On most targets, GCC supports implicit conversion between void * and function pointer types. However, for a portable solution, the concrete function pointer type needs to be used, together with explicit casts.

[marcstern]

Would a cast be sufficient (for both parameters)?
@Marcool04 Can you test this?

[Marcool04]

I've successfully tried the following patch which does allow modsecurity 2 to build on an up to date archlinux gcc14:

diff -ura modsecurity-2.9.7.orig/standalone/config.c modsecurity-2.9.7.mod/standalone/config.c
--- modsecurity-2.9.7.orig/standalone/config.c  2021-06-21 14:34:56.000000000 +0200
+++ modsecurity-2.9.7.mod/standalone/config.c   2024-10-04 07:29:48.346602243 +0200
@@ -1028,7 +1028,8 @@
        ap_directive_t *newdir;
        int optional;
        char *err = NULL;
-       char *rootpath, *incpath;
+       const char *rootpath, *incpath;
+       char *configfilepath;
        int li;

        errmsg = populate_include_files(p, ptemp, ari, filename, 0);
@@ -1108,13 +1109,13 @@

                                /* we allow APR_SUCCESS and APR_EINCOMPLETE */
                                if (APR_ERELATIVE == status) {
-                                       rootpath = apr_pstrdup(ptemp, parms->config_file->name);
-                                       li = strlen(rootpath) - 1;
+                                       configfilepath = apr_pstrdup(ptemp, parms->config_file->name);
+                                       li = strlen(configfilepath) - 1;

-                                       while(li >= 0 && rootpath[li] != '/' && rootpath[li] != '\\')
-                                               rootpath[li--] = 0;
+                                       while(li >= 0 && configfilepath[li] != '/' && configfilepath[li] != '\\')
+                                               configfilepath[li--] = 0;

-                                       w = apr_pstrcat(p, rootpath, w, NULL);
+                                       w = apr_pstrcat(p, configfilepath, w, NULL);
                                }
                                else if (APR_EBADPATH == status) {
                                        ap_cfg_closefile(parms->config_file);

In fact, as I was looking through config.c I noticed that here:

ModSecurity/standalone/config.c

Line 989 in 1121ef0

const char *rootpath, *filepath = fname;

rootpath was correctly cast as a const char before being passed to apr_filepath_root and that call didn't generate the error. I've also renamed the other completely unrelated use of rootpath to configfilepath but that might not be the best name, I don't know what variable naming principles you like to follow.

[marcstern]

LGTM.
Do you want to make a PR?

[Marcool04]

Submitted: #3270

[Marcool04]

Fixed in d7f2be6