diff --git a/.github/workflows/build-src.yml b/.github/workflows/build-src.yml index 8ea9ba443378..47a757d3941e 100644 --- a/.github/workflows/build-src.yml +++ b/.github/workflows/build-src.yml @@ -51,7 +51,9 @@ jobs: key: ${{ steps.bundle.outputs.key }} container: image: ${{ inputs.container-path }} - options: --user root + # SYS_PTRACE is required by LeakSanitizer's stop-the-world + # (https://github.com/google/sanitizers/issues/764) + options: --user root --cap-add SYS_PTRACE steps: - name: Checkout code uses: actions/checkout@v6 diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e8c993c6eeec..ce8191dc99ea 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -132,9 +132,9 @@ jobs: needs: [check-skip, container, cache-sources] if: | vars.SKIP_LINUX64 == '' || + vars.SKIP_LINUX64_ASAN == '' || vars.SKIP_LINUX64_FUZZ == '' || - vars.SKIP_LINUX64_SQLITE == '' || - vars.SKIP_LINUX64_UBSAN == '' + vars.SKIP_LINUX64_SQLITE == '' with: build-target: linux64 container-path: ${{ needs.container.outputs.path }} @@ -222,6 +222,20 @@ jobs: depends-artifact: ${{ needs.depends-linux64.outputs.built-artifact }} runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }} + src-linux64_asan: + name: linux64_asan-build + uses: ./.github/workflows/build-src.yml + needs: [check-skip, container, depends-linux64] + if: ${{ vars.SKIP_LINUX64_ASAN == '' }} + with: + build-target: linux64_asan + container-path: ${{ needs.container.outputs.path }} + depends-key: ${{ needs.depends-linux64.outputs.key }} + depends-host: ${{ needs.depends-linux64.outputs.host }} + depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }} + depends-artifact: ${{ needs.depends-linux64.outputs.built-artifact }} + runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }} + src-linux64_fuzz: name: linux64_fuzz-build uses: ./.github/workflows/build-src.yml @@ -291,20 +305,6 @@ jobs: depends-artifact: ${{ needs.depends-linux64_multiprocess.outputs.built-artifact }} runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }} - src-linux64_ubsan: - name: linux64_ubsan-build - uses: ./.github/workflows/build-src.yml - needs: [check-skip, container, depends-linux64] - if: ${{ vars.SKIP_LINUX64_UBSAN == '' }} - with: - build-target: linux64_ubsan - container-path: ${{ needs.container.outputs.path }} - depends-key: ${{ needs.depends-linux64.outputs.key }} - depends-host: ${{ needs.depends-linux64.outputs.host }} - depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }} - depends-artifact: ${{ needs.depends-linux64.outputs.built-artifact }} - runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }} - src-mac: name: mac-build uses: ./.github/workflows/build-src.yml @@ -342,6 +342,16 @@ jobs: container-path: ${{ needs.container-slim.outputs.path }} runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }} + test-linux64_asan: + name: linux64_asan-test + uses: ./.github/workflows/test-src.yml + needs: [check-skip, container-slim, src-linux64_asan, lint] + with: + bundle-key: ${{ needs.src-linux64_asan.outputs.key }} + build-target: linux64_asan + container-path: ${{ needs.container-slim.outputs.path }} + runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }} + test-linux64_multiprocess: name: linux64_multiprocess-test uses: ./.github/workflows/test-src.yml @@ -381,13 +391,3 @@ jobs: build-target: linux64_tsan container-path: ${{ needs.container-slim.outputs.path }} runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }} - - test-linux64_ubsan: - name: linux64_ubsan-test - uses: ./.github/workflows/test-src.yml - needs: [check-skip, container-slim, src-linux64_ubsan, lint] - with: - bundle-key: ${{ needs.src-linux64_ubsan.outputs.key }} - build-target: linux64_ubsan - container-path: ${{ needs.container-slim.outputs.path }} - runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }} diff --git a/.github/workflows/test-src.yml b/.github/workflows/test-src.yml index f27751be60a2..795a665f03e4 100644 --- a/.github/workflows/test-src.yml +++ b/.github/workflows/test-src.yml @@ -30,7 +30,9 @@ jobs: runs-on: ${{ inputs.runs-on }} container: image: ${{ inputs.container-path }} - options: --user root + # SYS_PTRACE is required by LeakSanitizer's stop-the-world + # (https://github.com/google/sanitizers/issues/764) + options: --user root --cap-add SYS_PTRACE steps: - name: Checkout code uses: actions/checkout@v6 diff --git a/ci/dash/matrix.sh b/ci/dash/matrix.sh index de01eaf6d6c7..224408b8e752 100755 --- a/ci/dash/matrix.sh +++ b/ci/dash/matrix.sh @@ -11,9 +11,9 @@ export LC_ALL=C.UTF-8 source ./ci/test/00_setup_env.sh # Configure sanitizers options -export ASAN_OPTIONS="" +export ASAN_OPTIONS="detect_leaks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1" export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/lsan" -export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1" +export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1:second_deadlock_stack=1" export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" if [ "$BUILD_TARGET" = "aarch64-linux" ]; then @@ -33,7 +33,11 @@ elif [ "$BUILD_TARGET" = "linux64_sqlite" ]; then elif [ "$BUILD_TARGET" = "linux64_tsan" ]; then source ./ci/test/00_setup_env_native_tsan.sh elif [ "$BUILD_TARGET" = "linux64_ubsan" ]; then - source ./ci/test/00_setup_env_native_ubsan.sh + # TODO: remove it when #7503 will get merged. That's a temporary workaround to check asan on CI + # Compatibility for pull_request_target workflows that still request the legacy target. + # Their default-branch container setup does not grant SYS_PTRACE. + export ASAN_OPTIONS="detect_leaks=0:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1" + source ./ci/test/00_setup_env_native_asan.sh elif [ "$BUILD_TARGET" = "linux64_valgrind" ]; then source ./ci/test/00_setup_env_native_valgrind.sh elif [ "$BUILD_TARGET" = "mac" ]; then diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 94e01cf906d4..c3029cfe68b4 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -6,12 +6,14 @@ export LC_ALL=C.UTF-8 -export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" -export NO_DEPENDS=1 -export TEST_RUNNER_EXTRA="--timeout-factor=4" # Increase timeout because sanitizers slow down -export FUNCTIONAL_TESTS_CONFIG="--exclude wallet_multiwallet.py" # Temporarily suppress ASan heap-use-after-free (see issue #14163) -export RUN_BENCH=true +export CONTAINER_NAME=ci_native_asan +export PACKAGES="clang-19 llvm-19 libclang-rt-19-dev python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" +# Reuses the depends built for the linux64 target, which uses the defaults. +export DEP_OPTS="" +export TEST_RUNNER_EXTRA="--timeout-factor=4 -j2" # Increase timeout because sanitizers slow down export GOAL="install" -export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 \ -CPPFLAGS=-DDEBUG_LOCKORDER \ ---with-sanitizers=address,float-divide-by-zero,integer,undefined CC=clang CXX=clang++" +export BITCOIN_CONFIG="--enable-zmq --enable-crash-hooks --with-gui=qt5 \ +--with-sanitizers=address,float-divide-by-zero,integer,undefined \ +CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' \ +CC='clang-19 -ftrivial-auto-var-init=pattern' CXX='clang++-19 -ftrivial-auto-var-init=pattern'" +export PYZMQ=true diff --git a/ci/test/00_setup_env_native_ubsan.sh b/ci/test/00_setup_env_native_ubsan.sh deleted file mode 100755 index f562bce0edbf..000000000000 --- a/ci/test/00_setup_env_native_ubsan.sh +++ /dev/null @@ -1,15 +0,0 @@ -#!/usr/bin/env bash -# -# Copyright (c) 2019 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -# TODO: use config `asan` instead `ubsan` for `undefined` sanitizer to unify with bitcoin -export LC_ALL=C.UTF-8 - -export CONTAINER_NAME=ci_native_ubsan -export PACKAGES="clang-19 llvm-19 python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" -export DEP_OPTS="NO_UPNP=1 DEBUG=1" -export GOAL="install" -export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --with-sanitizers=undefined CC=clang-19 CXX=clang++-19" -export PYZMQ=true diff --git a/configure.ac b/configure.ac index 1fcbafbd99fd..d36f8046cfa2 100644 --- a/configure.ac +++ b/configure.ac @@ -393,7 +393,8 @@ if test "$use_sanitizers" != ""; then dnl fail if a bad argument is passed, e.g. -fsanitize=undfeined AX_CHECK_COMPILE_FLAG( [-fsanitize=$use_sanitizers], - [SANITIZER_CXXFLAGS="-fsanitize=$use_sanitizers"], + [SANITIZER_CXXFLAGS="-fsanitize=$use_sanitizers" + SANITIZER_CFLAGS="-fsanitize=$use_sanitizers"], [AC_MSG_ERROR([compiler did not accept requested flags])]) dnl Some compilers (e.g. GCC) require additional libraries like libasan, @@ -2073,6 +2074,9 @@ CPPFLAGS_TEMP="$CPPFLAGS" unset CPPFLAGS CPPFLAGS="$CPPFLAGS_TEMP" +if test -n "$use_sanitizers"; then + export SECP_CFLAGS="$SECP_CFLAGS $SANITIZER_CFLAGS" +fi ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --disable-module-ecdh --disable-openssl-tests" AC_CONFIG_SUBDIRS([src/dashbls src/secp256k1]) @@ -2133,7 +2137,7 @@ echo " target os = $host_os" echo " build os = $build_os" echo echo " CC = $CC" -echo " CFLAGS = $BACKTRACE_FLAGS $DEBUG_CFLAGS $PTHREAD_CFLAGS $CFLAGS" +echo " CFLAGS = $SANITIZER_CFLAGS $BACKTRACE_FLAGS $DEBUG_CFLAGS $PTHREAD_CFLAGS $CFLAGS" echo " CPPFLAGS = $DEBUG_CPPFLAGS $HARDENED_CPPFLAGS $CORE_CPPFLAGS $CPPFLAGS" echo " CXX = $CXX" echo " CXXFLAGS = $BACKTRACE_FLAGS $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $CORE_CXXFLAGS $CXXFLAGS" diff --git a/contrib/containers/ci/ci-slim.Dockerfile b/contrib/containers/ci/ci-slim.Dockerfile index 8436fd14dfa3..10ece13d60ca 100644 --- a/contrib/containers/ci/ci-slim.Dockerfile +++ b/contrib/containers/ci/ci-slim.Dockerfile @@ -129,7 +129,9 @@ RUN set -ex; \ echo "deb [signed-by=/etc/apt/trusted.gpg.d/apt.llvm.org.asc] http://apt.llvm.org/${UBUNTU_CODENAME}/ llvm-toolchain-${UBUNTU_CODENAME}-${LLVM_VERSION} main" > /etc/apt/sources.list.d/llvm.list; \ apt-get update && apt-get install ${APT_ARGS} \ "llvm-${LLVM_VERSION}-dev"; \ - rm -rf /var/lib/apt/lists/*; + rm -rf /var/lib/apt/lists/*; \ + update-alternatives --install /usr/bin/llvm-symbolizer llvm-symbolizer \ + "/usr/bin/llvm-symbolizer-${LLVM_VERSION}" 100; # Setup unprivileged user and configuration files ARG USER_ID=1000 \ diff --git a/src/wallet/hdchain.cpp b/src/wallet/hdchain.cpp index 94b5e598d605..f781aaac075d 100644 --- a/src/wallet/hdchain.cpp +++ b/src/wallet/hdchain.cpp @@ -187,7 +187,8 @@ void CHDChain::AddAccount() bool CHDChain::GetAccount(uint32_t nAccountIndex, CHDAccount& hdAccountRet) { LOCK(cs); - if (nAccountIndex > mapAccounts.size() - 1) + // A chain stored without accounts creates one on first use, with zeroed counters. + if (!mapAccounts.empty() && nAccountIndex >= mapAccounts.size()) return false; hdAccountRet = mapAccounts[nAccountIndex]; return true; @@ -197,7 +198,7 @@ bool CHDChain::SetAccount(uint32_t nAccountIndex, const CHDAccount& hdAccount) { LOCK(cs); // can only replace existing accounts - if (nAccountIndex > mapAccounts.size() - 1) + if (!mapAccounts.empty() && nAccountIndex >= mapAccounts.size()) return false; mapAccounts[nAccountIndex] = hdAccount; return true; diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index 6a1bd06f5b99..2b88b69cea33 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -255,7 +255,7 @@ bool CZMQPublishHashChainLockNotifier::NotifyChainLock(const CBlockIndex *pindex { uint256 hash = pindex->GetBlockHash(); LogPrint(BCLog::ZMQ, "Publish hashchainlock %s to %s\n", hash.GetHex(), this->address); - char data[32]; + uint8_t data[32]; for (unsigned int i = 0; i < 32; i++) data[31 - i] = hash.begin()[i]; return SendZmqMessage(MSG_HASHCHAINLOCK, data, 32); @@ -276,7 +276,7 @@ bool CZMQPublishHashTransactionLockNotifier::NotifyTransactionLock(const CTransa { uint256 hash = transaction->GetHash(); LogPrint(BCLog::ZMQ, "Publish hashtxlock %s to %s\n", hash.GetHex(), this->address); - char data[32]; + uint8_t data[32]; for (unsigned int i = 0; i < 32; i++) data[31 - i] = hash.begin()[i]; return SendZmqMessage(MSG_HASHTXLOCK, data, 32); @@ -286,7 +286,7 @@ bool CZMQPublishHashGovernanceVoteNotifier::NotifyGovernanceVote(const std::shar { uint256 hash = vote->GetHash(); LogPrint(BCLog::ZMQ, "Publish hashgovernancevote %s to %s\n", hash.GetHex(), this->address); - char data[32]; + uint8_t data[32]; for (unsigned int i = 0; i < 32; i++) data[31 - i] = hash.begin()[i]; return SendZmqMessage(MSG_HASHGVOTE, data, 32); @@ -296,7 +296,7 @@ bool CZMQPublishHashGovernanceObjectNotifier::NotifyGovernanceObject(const std:: { uint256 hash = object->GetHash(); LogPrint(BCLog::ZMQ, "Publish hashgovernanceobject %s to %s\n", hash.GetHex(), this->address); - char data[32]; + uint8_t data[32]; for (unsigned int i = 0; i < 32; i++) data[31 - i] = hash.begin()[i]; return SendZmqMessage(MSG_HASHGOBJ, data, 32); @@ -306,7 +306,7 @@ bool CZMQPublishHashInstantSendDoubleSpendNotifier::NotifyInstantSendDoubleSpend { uint256 currentHash = currentTx->GetHash(), previousHash = previousTx->GetHash(); LogPrint(BCLog::ZMQ, "Publish hashinstantsenddoublespend %s conflicts against %s to %s\n", currentHash.ToString(), previousHash.ToString(), this->address); - char dataCurrentHash[32], dataPreviousHash[32]; + uint8_t dataCurrentHash[32], dataPreviousHash[32]; for (unsigned int i = 0; i < 32; i++) { dataCurrentHash[31 - i] = currentHash.begin()[i]; dataPreviousHash[31 - i] = previousHash.begin()[i]; @@ -318,7 +318,7 @@ bool CZMQPublishHashInstantSendDoubleSpendNotifier::NotifyInstantSendDoubleSpend bool CZMQPublishHashRecoveredSigNotifier::NotifyRecoveredSig(const std::shared_ptr &sig) { LogPrint(BCLog::ZMQ, "Publish hashrecoveredsig %s to %s\n", sig->getMsgHash().ToString(), this->address); - char data[32]; + uint8_t data[32]; for (unsigned int i = 0; i < 32; i++) data[31 - i] = sig->getMsgHash().begin()[i]; return SendZmqMessage(MSG_HASHRECSIG, data, 32); diff --git a/test/sanitizer_suppressions/lsan b/test/sanitizer_suppressions/lsan index 7ccb22515f91..fc1d82f45976 100644 --- a/test/sanitizer_suppressions/lsan +++ b/test/sanitizer_suppressions/lsan @@ -1,2 +1,4 @@ # Suppress warnings triggered in dependencies leak:libQt5Widgets +leak:QDBusConnectionPrivate +leak:QLayoutPrivate diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index b486e22bea12..5560aada773a 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -12,25 +12,27 @@ unsigned-integer-overflow:*/include/c++/ unsigned-integer-overflow:FuzzedDataProvider::ConsumeIntegralInRange unsigned-integer-overflow:leveldb/ unsigned-integer-overflow:minisketch/ -unsigned-integer-overflow:secp256k1/ +unsigned-integer-overflow:secp256k1* unsigned-integer-overflow:test/fuzz/crypto_diff_fuzz_chacha20.cpp implicit-integer-sign-change:*/include/boost/ implicit-integer-sign-change:*/include/c++/ implicit-integer-sign-change:*/new_allocator.h implicit-integer-sign-change:crc32c/ implicit-integer-sign-change:minisketch/ -implicit-integer-sign-change:secp256k1/ +implicit-integer-sign-change:secp256k1* implicit-signed-integer-truncation:*/include/c++/ implicit-signed-integer-truncation:leveldb/ -implicit-signed-integer-truncation:secp256k1/ +implicit-signed-integer-truncation:secp256k1* +implicit-signed-integer-truncation,implicit-integer-sign-change:secp256k1_modinv64_posdivsteps_62_var implicit-unsigned-integer-truncation:*/include/c++/ implicit-unsigned-integer-truncation:leveldb/ -implicit-unsigned-integer-truncation:secp256k1/ +implicit-unsigned-integer-truncation:secp256k1* +implicit-signed-integer-truncation,implicit-integer-sign-change:secp256k1* implicit-unsigned-integer-truncation:test/fuzz/crypto_diff_fuzz_chacha20.cpp shift-base:*/include/c++/ shift-base:leveldb/ shift-base:minisketch/ -shift-base:secp256k1/ +shift-base:secp256k1* shift-base:test/fuzz/crypto_diff_fuzz_chacha20.cpp # Unsigned integer overflow occurs when the result of an unsigned integer # computation cannot be represented in its type. Unlike signed integer overflow, @@ -46,6 +48,13 @@ unsigned-integer-overflow:compressor.cpp unsigned-integer-overflow:crypto/ unsigned-integer-overflow:hash.cpp unsigned-integer-overflow:lcg.h +# The quorum snapshot skip list stores offsets relative to the first skipped +# index, and the index wraps around the sorted MN list, so an offset can be +# negative. Encoding wraps size_t and narrows into vector, decoding wraps +# it back; the two cancel and the absolute index is recovered exactly. Per +# symbol rather than per file so the rest of llmq/utils.cpp stays covered. +unsigned-integer-overflow:BuildNewQuorumQuarterMembers +unsigned-integer-overflow:GetQuorumQuarterMembersBySnapshot unsigned-integer-overflow:policy/fees.cpp unsigned-integer-overflow:prevector.h unsigned-integer-overflow:EvalScript @@ -54,6 +63,7 @@ unsigned-integer-overflow:xoroshiro128plusplus.h implicit-integer-sign-change:addrman.h implicit-integer-sign-change:compat/stdin.cpp implicit-integer-sign-change:CBlockPolicyEstimator::processBlockTx +implicit-integer-sign-change:GetQuorumQuarterMembersBySnapshot implicit-integer-sign-change:compressor.h implicit-integer-sign-change:crypto/ implicit-integer-sign-change:policy/fees.cpp @@ -66,6 +76,7 @@ implicit-integer-sign-change:util/strencodings.cpp implicit-integer-sign-change:util/strencodings.h implicit-integer-sign-change:validation.cpp implicit-signed-integer-truncation,implicit-integer-sign-change:test/skiplist_tests.cpp +implicit-signed-integer-truncation,implicit-integer-sign-change:BuildNewQuorumQuarterMembers implicit-signed-integer-truncation:addrman.h implicit-signed-integer-truncation:crypto/ implicit-unsigned-integer-truncation:crypto/