1905746 - Numerous cases of UB due to calling a function through a pointer of incorrect type

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[John Schanck [:jschanck]]

In recent versions of Clang, the -fsanitize=undefined option implies -fsanitize=function. This detects numerous cases of undefined behavior in NSS. For example:

../../lib/softoken/pkcs11c.c:2009:5: runtime error: call to function SHA1_Update through pointer to incorrect function type 'void (*)(void *, const void *, unsigned int)'
/home/worker/nss/out/Debug/../../lib/freebl/sha_fast.c:126: note: SHA1_Update defined here
    #0 0x5601267fffff in NSC_DigestUpdate /home/worker/nss/out/Debug/../../lib/softoken/pkcs11c.c:2009:5

This is blocking our ability to upgrade clang in our CI.

Comment 1

[John Schanck [:jschanck]]

Attachment: Bug 1905746 - avoid calling functions through pointers of incompatible type. r=#nss-reviewers

Comment 2

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/1f2370c70b4a7401cf7ef5784cc130cfa22a1416

Comment 4

[Lars Eggert [:lars]]

Attachment: error messages

-

Comment 5

[Lars Eggert [:lars]]

The fix for this causes build breakage of the neqo docker image on ARM64 macOS:

7.748 cc -MMD -MF obj/lib/freebl/freeblpriv3.rijndael.o.d '-DSHLIB_SUFFIX="so"' '-DSHLIB_PREFIX="lib"' '-DSHLIB_VERSION="3"' '-DSOFTOKEN_SHLIB_VERSION="3"' -DRIJNDAEL_INCLUDE_TABLES -DMP_API_COMPATIBLE -DHAVE_INT128_SUPPORT -DHACL_CAN_COMPILE_UINT128 -DFREEBL_LOWHASH -DFREEBL_NO_DEPEND -DUSE_HW_AES -DUSE_HW_SHA1 -DUSE_HW_SHA2 -DNSS_FIPS_DISABLED -DNSS_NO_INIT_SUPPORT -DNSS_USE_64 -DKYBER_K=3 -DSEED_ONLY_DEV_URANDOM -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DLINUX2_1 -DLINUX -Dlinux -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_POSIX_SOURCE -DSDB_MEASURE_USE_TEMP_DIR -DHAVE_STRERROR -DXP_UNIX -D_REENTRANT -DNSS_DISABLE_DBM -DNSS_DISABLE_LIBPKIX -DNDEBUG -I../../lib/freebl/mpi -I../../lib/freebl/ecl -I../../lib/freebl/verified -I../../lib/freebl/verified/internal -I../../lib/freebl/verified/karamel/include -I../../lib/freebl/verified/karamel/krmllib/dist/minimal -I../../lib/freebl/deprecated -I/dist/Release/include/nspr -I/dist/private/nss -I/dist/public/nss -fPIC -pipe -ffunction-sections -fdata-sections -Werror -Wall -Wshadow -O2 -std=c99  -c ../../lib/freebl/rijndael.c -o obj/lib/freebl/freeblpriv3.rijndael.o
7.748 ../../lib/freebl/rijndael.c: In function 'freeblCipher_intel_aes_encrypt_ecb_128':
7.748 ../../lib/freebl/rijndael.c:889:32: error: implicit declaration of function 'intel_aes_encrypt_ecb_128'; did you mean 'arm_aes_encrypt_ecb_128'? [-Werror=implicit-function-declaration]
7.748   889 | FREEBL_CIPHER_WRAP(AESContext, intel_aes_encrypt_ecb_128);
7.748       |                                ^~~~~~~~~~~~~~~~~~~~~~~~~
7.748 ../../lib/freebl/rijndael.c:867:16: note: in definition of macro 'FREEBL_CIPHER_WRAP'
7.748   867 |         return mmm(ctx, output, outputLen, maxOutputLen, input, inputLen, blocksize);       \
7.748       |                ^~~
7.748 ../../lib/freebl/rijndael.c: In function 'freeblCipher_intel_aes_decrypt_ecb_128':
7.748 ../../lib/freebl/rijndael.c:890:32: error: implicit declaration of function 'intel_aes_decrypt_ecb_128'; did you mean 'arm_aes_decrypt_ecb_128'? [-Werror=implicit-function-declaration]
7.748   890 | FREEBL_CIPHER_WRAP(AESContext, intel_aes_decrypt_ecb_128);
7.748       |                                ^~~~~~~~~~~~~~~~~~~~~~~~~
7.748 ../../lib/freebl/rijndael.c:867:16: note: in definition of macro 'FREEBL_CIPHER_WRAP'
7.748   867 |         return mmm(ctx, output, outputLen, maxOutputLen, input, inputLen, blocksize);       \
7.748       |                ^~~

(and many more like it).

Comment 6

[Lars Eggert [:lars]]

The build is also broken from source on macOS.

 lars  ~/D/C/n/nss    ./build.sh --disable-tests -o
ninja: Entering directory `/Users/lars/Documents/Code/nss/nss/out/Release'
[25/283] CC obj/lib/freebl/freebl_static.rijndael.o
FAILED: obj/lib/freebl/freebl_static.rijndael.o
cc -MMD -MF obj/lib/freebl/freebl_static.rijndael.o.d '-DSHLIB_SUFFIX="dylib"' '-DSHLIB_PREFIX="lib"' '-DSHLIB_VERSION="3"' '-DSOFTOKEN_SHLIB_VERSION="3"' -DRIJNDAEL_INCLUDE_TABLES -DMP_API_COMPATIBLE -DUSE_HW_AES -DUSE_HW_SHA1 -DUSE_HW_SHA2 -DHAVE_INT128_SUPPORT -DHACL_CAN_COMPILE_UINT128 -DNSS_FIPS_DISABLED -DNSS_NO_INIT_SUPPORT -DNSS_USE_64 -DKYBER_K=3 -DSEED_ONLY_DEV_URANDOM -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DHAVE_BSD_FLOCK -DHAVE_STRERROR -DXP_UNIX -D_REENTRANT -DDARWIN -DNSS_DISABLE_DBM -DNSS_DISABLE_LIBPKIX -DNDEBUG -I../../lib/freebl/mpi -I../../lib/freebl/ecl -I../../lib/freebl/verified -I../../lib/freebl/verified/internal -I../../lib/freebl/verified/karamel/include -I../../lib/freebl/verified/karamel/krmllib/dist/minimal -I../../lib/freebl/deprecated -I/Users/lars/Documents/Code/nss/dist/Release/include/nspr -I/Users/lars/Documents/Code/nss/dist/private/nss -I/Users/lars/Documents/Code/nss/dist/public/nss -fasm-blocks -mpascal-strings -O2 -gdwarf-2 -arch arm64 -fPIC -fno-common -pipe -Werror -Wall -Wno-array-bounds -Wno-unevaluated-expression -Wno-parentheses-equality -Wno-tautological-type-limit-compare -Wno-sign-compare -Wno-comma -Wno-implicit-fallthrough -Wtautological-constant-in-range-compare -Wbitfield-enum-conversion -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wimplicit-function-declaration -Qunused-arguments -Wshadow  -c ../../lib/freebl/rijndael.c -o obj/lib/freebl/freebl_static.rijndael.o
../../lib/freebl/rijndael.c:889:32: error: call to undeclared function 'intel_aes_encrypt_ecb_128'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
FREEBL_CIPHER_WRAP(AESContext, intel_aes_encrypt_ecb_128);
                               ^
../../lib/freebl/rijndael.c:889:32: note: did you mean 'arm_aes_encrypt_ecb_128'?
../../lib/freebl/aes-armv8.h:5:11: note: 'arm_aes_encrypt_ecb_128' declared here
SECStatus arm_aes_encrypt_ecb_128(AESContext *cx, unsigned char *output,
          ^

Comment 7

[John Schanck [:jschanck]]

Thanks for flagging this, Lars. The patch in Bug 1908190 should fix it (try run).