nbs-system / naxsi

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

compilation on Ubuntu 20.04 fails #491

Closed BWC-Michael closed 4 years ago

BWC-Michael commented 4 years ago

Trying to compile nginx 1.18 with naxsi 0.56 on Ubuntu 20.04, but it fails, no probs on Ubuntu 18.04. Is this related to #480?

gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0

cc -c -fPIC -pipe  -O -W -Wall -Wpointer-arith -Wno-unused-parameter -Werror -g -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -I src/core -I src/event -I src/event/modules -I src/os/unix -I objs -I src/http -I src/http/modules -I src/http/v2 \
    -o objs/addon/naxsi_src/naxsi_skeleton.o \
    ../naxsi-0.56//naxsi_src//naxsi_skeleton.c
In file included from /usr/include/string.h:495,
                 from src/os/unix/ngx_linux_config.h:27,
                 from src/core/ngx_config.h:26,
                 from ../naxsi-0.56//naxsi_src//naxsi.h:38,
                 from ../naxsi-0.56//naxsi_src//naxsi_skeleton.c:37:
In function ‘strncpy’,
    inlined from ‘ngx_http_dummy_init’ at ../naxsi-0.56//naxsi_src//naxsi_skeleton.c:417:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output truncated before terminating nul copying 17 bytes from a string of the same length [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
    inlined from ‘ngx_http_dummy_init’ at ../naxsi-0.56//naxsi_src//naxsi_skeleton.c:418:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output truncated before terminating nul copying 17 bytes from a string of the same length [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [objs/Makefile:1425: objs/addon/naxsi_src/naxsi_skeleton.o] Error 1
petecooper commented 4 years ago

Similar compile error here:

-o objs/addon/naxsi_src/naxsi_skeleton.o \
    ../../naxsi-source/naxsi-0.56/naxsi_src/naxsi_skeleton.c
In file included from /usr/include/string.h:495,
                 from src/os/unix/ngx_linux_config.h:27,
                 from src/core/ngx_config.h:26,
                 from ../../naxsi-source/naxsi-0.56/naxsi_src/naxsi.h:38,
                 from ../../naxsi-source/naxsi-0.56/naxsi_src/naxsi_skeleton.c:37:
In function ‘strncpy’,
    inlined from ‘ngx_http_dummy_init’ at ../../naxsi-source/naxsi-0.56/naxsi_src/naxsi_skeleton.c:417:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘strncpy’ output truncated before terminating nul copying 17 bytes from a string of the same length [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
    inlined from ‘ngx_http_dummy_init’ at ../../naxsi-source/naxsi-0.56/naxsi_src/naxsi_skeleton.c:418:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘strncpy’ output truncated before terminating nul copying 17 bytes from a string of the same length [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [objs/Makefile:1998: objs/addon/naxsi_src/naxsi_skeleton.o] Error 1
make[1]: Leaving directory '/home/pete/nginx-source/nginx-1.17.10'
make: *** [Makefile:8: build] Error 2
marcinguy commented 4 years ago

@BWC-Michael @petecooper

Check out this PR: https://github.com/nbs-system/naxsi/pull/492

You have to change 2 lines: https://github.com/nbs-system/naxsi/pull/492/files

Was able to compile it then.

Take care,

marcinguy commented 4 years ago

Actually modified my PR to use memcpy()

https://github.com/nbs-system/naxsi/pull/492/commits/0cda8686d4dc2c06db563335934527dc09bf3b3b

Let's wait for Author's Review. It should now break anything, but you never know.

Take care,

CC @buixor

petecooper commented 4 years ago

I tested https://github.com/nbs-system/naxsi/pull/492/commits/0cda8686d4dc2c06db563335934527dc09bf3b3b - working here, compiles without errors.

See https://github.com/nbs-system/naxsi/pull/492#issuecomment-620576012

marcinguy commented 4 years ago

Actually the first commit is correct. I see 18 bytes are allocated, so this includes the null character.

https://github.com/nbs-system/naxsi/blob/f73b525f294de00a4f777e6b495c798d7c0d6ad4/naxsi_src/naxsi_skeleton.c#L390

I will change it, cancel second commit, when I have time.

Cc @buixor

petecooper commented 4 years ago

One-liner for anyone using Naxsi v0.56 (not current source) with Nginx 1.17 & 1.18 on Ubuntu 20.04, change the location of naxsi_src/naxsi_skeleton.c to your own:

sed -i '417,418s/strncpy/memcpy/' ./naxsi_src/naxsi_skeleton.c

Tested and working here, feedback is welcomed.

BWC-Michael commented 4 years ago

With the latest one-liner applied I was able to compile nginx 1.18 and Naxsi 0.56 as dynamic module without any errors.

ston1th commented 4 years ago

Hi @wargio

I tried to compile v0.56 on alpine 3.11 and got these errors:

/tmp/build/naxsi-0.56/naxsi_src/naxsi_runtime.c: In function 'ngx_http_dummy_is_rule_whitelisted_n':
/tmp/build/naxsi-0.56/naxsi_src/naxsi_runtime.c:728:5: error: 'strncat' specified bound 1 equals source length [-Werror=stringop-overflow=]
  728 |     strncat((char*)tmp_hashname.data, "#", 1);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/build/naxsi-0.56/naxsi_src/naxsi_runtime.c:731:3: error: 'strncat' specified bound 1 equals source length [-Werror=stringop-overflow=]
  731 |   strncat((char*)tmp_hashname.data, "#", 1);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/build/naxsi-0.56/naxsi_src/naxsi_utils.c: In function 'ngx_http_wlr_find':
/tmp/build/naxsi-0.56/naxsi_src/naxsi_utils.c:381:7: error: 'strncat' specified bound 1 equals source length [-Werror=stringop-overflow=]
  381 |       strncat(*fullname, (const char *) "#", 1);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/build/naxsi-0.56/naxsi_src/naxsi_utils.c:385:5: error: 'strncat' specified bound 1 equals source length [-Werror=stringop-overflow=]
  385 |     strncat(*fullname, (const char *) "#", 1);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/build/naxsi-0.56/naxsi_src/naxsi_utils.c:403:7: error: 'strncat' specified bound 1 equals source length [-Werror=stringop-overflow=]
  403 |       strncat(*fullname, (const char *) "#", 1);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/build/naxsi-0.56/naxsi_src/naxsi_utils.c:418:7: error: 'strncat' specified bound 1 equals source length [-Werror=stringop-overflow=]
  418 |       strncat(*fullname, (const char *) "#", 1);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I fixed this by replacing the size of 1 by 2.

marcinguy commented 4 years ago

@wargio @ston1th It seems like the warning message is pointing out a misuse of strncat(). Specifying the length of src on 3rd argument is the same as calling strcat()

You can change it to strcat() or memcpy() and add null termination.

What you did with 2 will create null-terminated 3 chars length string (inc. null terminator), not sure, but possibly garbage 2nd char, possibly 0. So it can kinda work. Not sure.

If you can't change the code, use -Wno-stringop-overflow flag to disable the warning.

Made a PR to change to strcat():

https://github.com/nbs-system/naxsi/pull/497

The build is passing. Hopefully the above works on your "Alpine" image.

marcinguy commented 4 years ago

FYI @wargio @ston1th Made a simple test to see how it behaves on 64 bit system:

x/10s $rax 0x7fffffffd993: "#" 0x7fffffffd995: "" 0x7fffffffd996: "" 0x7fffffffd997: ""

0x7fffffffd993: 0x23 0x00 0x00 0x00 0x00

Actually seems like allocated memory will have 0x00 (will be null terminated in that case).

you string will be 0x23 0x00 0x00 with using 2 as a 3rd argument.

If I understood it correctly.

So your code will also I guess work.

wargio commented 4 years ago

actually the warning is correct. that len field should refer to the size of the source string. adding +1 is correct. I'll try next week to fix this.

wargio commented 4 years ago

should be fixed via https://github.com/nbs-system/naxsi/pull/497