From df76002a446ec416c6097c6ef3b475f4d2fb1005 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Fri, 23 Jan 2026 15:39:21 +0000 Subject: [PATCH] fixed asan issues Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- .config/cspell.config.yaml | 2 + .github/scripts/strategy-matrix/generate.py | 10 +++-- .../workflows/reusable-build-test-config.yml | 2 +- conanfile.py | 7 +++ include/xrpl/net/HTTPClient.h | 3 ++ sanitizers/suppressions/asan.supp | 43 ++++++++----------- sanitizers/suppressions/lsan.supp | 18 +++----- sanitizers/suppressions/ubsan.supp | 1 + src/libxrpl/net/HTTPClient.cpp | 6 +++ src/tests/libxrpl/net/HTTPClient.cpp | 6 +++ 10 files changed, 57 insertions(+), 41 deletions(-) diff --git a/.config/cspell.config.yaml b/.config/cspell.config.yaml index 73b0417d79..7b6688938f 100644 --- a/.config/cspell.config.yaml +++ b/.config/cspell.config.yaml @@ -89,6 +89,7 @@ words: - endmacro - exceptioned - Falco + - fcontext - finalizers - firewalled - fmtdur @@ -213,6 +214,7 @@ words: - soci - socidb - sslws + - stackful - statsd - STATSDCOLLECTOR - stissue diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index 0e44b1be54..8282f14ea6 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -242,10 +242,12 @@ def generate_strategy_matrix(all: bool, config: Config) -> list: # names get truncated. # Add Address and Thread (both coupled with UB) sanitizers for specific bookworm distros. # GCC-Asan rippled-embedded tests are failing because of https://github.com/google/sanitizers/issues/856 - if ( - os["distro_version"] == "bookworm" - and f"{os['compiler_name']}-{os['compiler_version']}" == "clang-20" - ): + if os[ + "distro_version" + ] == "bookworm" and f"{os['compiler_name']}-{os['compiler_version']}" in [ + "clang-20", + "gcc-13", + ]: # Add ASAN + UBSAN configuration. configurations.append( { diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index d298c85726..2f6426a43e 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -205,7 +205,7 @@ jobs: - name: Set sanitizer options if: ${{ !inputs.build_only && env.SANITIZERS_ENABLED == 'true' }} run: | - echo "ASAN_OPTIONS=print_stacktrace=1:detect_container_overflow=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" >> ${GITHUB_ENV} + echo "ASAN_OPTIONS=print_stacktrace=1:detect_container_overflow=0:detect_stack_use_after_return=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp:halt_on_error=0" >> ${GITHUB_ENV} echo "TSAN_OPTIONS=second_deadlock_stack=1:halt_on_error=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV} echo "UBSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV} echo "LSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV} diff --git a/conanfile.py b/conanfile.py index 8501909ce3..f8222e9c3b 100644 --- a/conanfile.py +++ b/conanfile.py @@ -1,4 +1,5 @@ import re +import os from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout @@ -126,6 +127,12 @@ class Xrpl(ConanFile): if self.settings.compiler in ["clang", "gcc"]: self.options["boost"].without_cobalt = True + # Check if environment variable exists + if "SANITIZERS" in os.environ: + sanitizers = os.environ["SANITIZERS"] + if "Address" in sanitizers: + self.default_options["fPIC"] = False + def requirements(self): # Conan 2 requires transitive headers to be specified transitive_headers_opt = ( diff --git a/include/xrpl/net/HTTPClient.h b/include/xrpl/net/HTTPClient.h index 6e4d4ce685..f8461e0588 100644 --- a/include/xrpl/net/HTTPClient.h +++ b/include/xrpl/net/HTTPClient.h @@ -30,6 +30,9 @@ public: bool sslVerify, beast::Journal j); + static void + cleanupSSLContext(); + static void get(bool bSSL, boost::asio::io_context& io_context, diff --git a/sanitizers/suppressions/asan.supp b/sanitizers/suppressions/asan.supp index 7ba98766bd..73a44f3b71 100644 --- a/sanitizers/suppressions/asan.supp +++ b/sanitizers/suppressions/asan.supp @@ -1,29 +1,22 @@ -# The idea is to empty this file gradually by fixing the underlying issues and removing suppressions. +# The idea is to empty this file gradually by fixing the underlying issues and removing suppresions. # -# ASAN_OPTIONS="print_stacktrace=1:detect_container_overflow=0:suppressions=sanitizers/suppressions/asan.supp:halt_on_error=0" +# ASAN_OPTIONS="suppressions=sanitizers/suppressions/asan.supp:halt_on_error=0:detect_stack_use_after_return=0" # -# The detect_container_overflow=0 option disables false positives from: -# - Boost intrusive containers (slist_iterator.hpp, hashtable.hpp, aged_unordered_container.h) -# - Boost context/coroutine stack switching (Workers.cpp, thread.h) +# Boost coroutines cause multiple ASAN false positives due to swapcontext/fiber stack switching. +# ASAN cannot correctly track stack memory across coroutine context switches, leading to: +# - stack-use-after-return errors +# - stack-use-after-scope errors +# - stack-buffer-overflow errors in seemingly unrelated code (e.g., std::chrono::steady_clock::now()) # -# See: https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow +# These are suppressed via: +# 1. Runtime option: detect_stack_use_after_return=0 (in ASAN_OPTIONS in CI workflow) +# 2. Compile-time flag: -fno-sanitize-address-use-after-scope (in cmake/XrplSanitizers.cmake) +# +# Note: stack-buffer-overflow false positives from coroutines cannot be fully suppressed +# without disabling ASAN entirely for Boost. Clang builds use -fsanitize-blacklist to +# exclude Boost headers, but GCC does not support this feature. +# +# See: https://github.com/google/sanitizers/issues/189 -# Boost -interceptor_name:boost/asio - -# Leaks in Doctest tests: xrpl.test.* -interceptor_name:src/libxrpl/net/HTTPClient.cpp -interceptor_name:src/libxrpl/net/RegisterSSLCerts.cpp -interceptor_name:src/tests/libxrpl/net/HTTPClient.cpp -interceptor_name:xrpl/net/AutoSocket.h -interceptor_name:xrpl/net/HTTPClient.h -interceptor_name:xrpl/net/HTTPClientSSLContext.h -interceptor_name:xrpl/net/RegisterSSLCerts.h - -# Suppress false positive stack-buffer errors in thread stack allocation -# Related to ASan's __asan_handle_no_return warnings (github.com/google/sanitizers/issues/189) -# These occur during multi-threaded test initialization on macOS -interceptor_name:memcpy -interceptor_name:__bzero -interceptor_name:__asan_memset -interceptor_name:__asan_memcpy +# Boost - false positives from stackful coroutines +interceptor_name:boost diff --git a/sanitizers/suppressions/lsan.supp b/sanitizers/suppressions/lsan.supp index a81d7d89fa..a0f5569a57 100644 --- a/sanitizers/suppressions/lsan.supp +++ b/sanitizers/suppressions/lsan.supp @@ -1,16 +1,12 @@ # The idea is to empty this file gradually by fixing the underlying issues and removing suppresions. -# Suppress leaks detected by asan in rippled code. -leak:src/libxrpl/net/HTTPClient.cpp -leak:src/libxrpl/net/RegisterSSLCerts.cpp -leak:src/tests/libxrpl/net/HTTPClient.cpp -leak:xrpl/net/AutoSocket.h -leak:xrpl/net/HTTPClient.h -leak:xrpl/net/HTTPClientSSLContext.h -leak:xrpl/net/RegisterSSLCerts.h -leak:ripple::HTTPClient -leak:ripple::HTTPClientImp - # Suppress leaks detected by asan in boost code. +# These are false positives from Boost.Asio SSL internals that use OpenSSL BIO structures. +# The BIO structures are managed by OpenSSL's internal reference counting and freed at process exit. leak:boost::asio leak:boost/asio + +# OpenSSL BIO memory is managed internally and freed at process exit +leak:CRYPTO_malloc +leak:bio_make_pair +leak:BIO_new_bio_pair diff --git a/sanitizers/suppressions/ubsan.supp b/sanitizers/suppressions/ubsan.supp index 1504ef685f..dc821f8d30 100644 --- a/sanitizers/suppressions/ubsan.supp +++ b/sanitizers/suppressions/ubsan.supp @@ -140,6 +140,7 @@ unsigned-integer-overflow:src/libxrpl/protocol/tokens.cpp unsigned-integer-overflow:src/libxrpl/shamap/SHAMap.cpp unsigned-integer-overflow:src/test/app/Batch_test.cpp unsigned-integer-overflow:src/test/app/Invariants_test.cpp +unsigned-integer-overflow:src/test/app/Loan_test.cpp unsigned-integer-overflow:src/test/app/NFToken_test.cpp unsigned-integer-overflow:src/test/app/Offer_test.cpp unsigned-integer-overflow:src/test/app/Path_test.cpp diff --git a/src/libxrpl/net/HTTPClient.cpp b/src/libxrpl/net/HTTPClient.cpp index 6057d81a1e..e550893e9a 100644 --- a/src/libxrpl/net/HTTPClient.cpp +++ b/src/libxrpl/net/HTTPClient.cpp @@ -26,6 +26,12 @@ HTTPClient::initializeSSLContext( httpClientSSLContext.emplace(sslVerifyDir, sslVerifyFile, sslVerify, j); } +void +HTTPClient::cleanupSSLContext() +{ + httpClientSSLContext.reset(); +} + //------------------------------------------------------------------------------ // // Fetch a web page via http or https. diff --git a/src/tests/libxrpl/net/HTTPClient.cpp b/src/tests/libxrpl/net/HTTPClient.cpp index cfd206edde..c1906d8c78 100644 --- a/src/tests/libxrpl/net/HTTPClient.cpp +++ b/src/tests/libxrpl/net/HTTPClient.cpp @@ -254,6 +254,9 @@ TEST(HTTPClient, case_insensitive_content_length) EXPECT_EQ(result_status, 200); EXPECT_EQ(result_data, test_body); } + + // Clean up SSL context to prevent memory leaks + HTTPClient::cleanupSSLContext(); } TEST(HTTPClient, basic_http_request) @@ -324,4 +327,7 @@ TEST(HTTPClient, different_status_codes) EXPECT_FALSE(result_error); EXPECT_EQ(result_status, static_cast(status)); } + + // Clean up SSL context to prevent memory leaks + HTTPClient::cleanupSSLContext(); }