From 817234fa94538383a8957561bec5c4e073cd7d3a Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Wed, 18 Mar 2026 16:25:32 -0500 Subject: [PATCH 1/2] fix: resolve signed integer overflow UB in CoinJoin priority and timeout CalculateAmountPriority in common.h could overflow when assigning a negated int64_t division result to an int return type with extreme CAmount values. Add a MoneyRange guard to return 0 for out-of-range inputs, as CoinJoin amounts are always within valid money range. IsTimeOutOfBounds in coinjoin.cpp could overflow on signed subtraction when current_time and nTime are extreme values. Add a guard rejecting negative timestamps (which are always invalid) so the original subtraction logic is safe for all remaining non-negative inputs. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/coinjoin/coinjoin.cpp | 1 + src/coinjoin/common.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 6391e73f2bc5..9b4d8953c48f 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -57,6 +57,7 @@ bool CCoinJoinQueue::CheckSignature(const CBLSPublicKey& blsPubKey) const bool CCoinJoinQueue::IsTimeOutOfBounds(int64_t current_time) const { + if (current_time < 0 || nTime < 0) return true; return current_time - nTime > COINJOIN_QUEUE_TIMEOUT || nTime - current_time > COINJOIN_QUEUE_TIMEOUT; } diff --git a/src/coinjoin/common.h b/src/coinjoin/common.h index 6c7abfba2266..3d97d639434d 100644 --- a/src/coinjoin/common.h +++ b/src/coinjoin/common.h @@ -118,6 +118,7 @@ constexpr bool IsCollateralAmount(CAmount nInputAmount) constexpr int CalculateAmountPriority(CAmount nInputAmount) { + if (nInputAmount < 0 || nInputAmount > MAX_MONEY) return 0; if (auto optDenom = util::find_if_opt(GetStandardDenominations(), [&nInputAmount](const auto& denom) { return nInputAmount == denom; })) { return (float)COIN / *optDenom * 10000; From e8ec63a1e090f4e6bcdfa3cd48891c8672b51433 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Thu, 19 Mar 2026 21:59:44 -0500 Subject: [PATCH 2/2] test: add regression tests for CoinJoin UB fixes Co-Authored-By: Claude Opus 4.6 (1M context) --- src/test/coinjoin_queue_tests.cpp | 49 +++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/test/coinjoin_queue_tests.cpp b/src/test/coinjoin_queue_tests.cpp index f43d8ecdf229..a39ad552dbc2 100644 --- a/src/test/coinjoin_queue_tests.cpp +++ b/src/test/coinjoin_queue_tests.cpp @@ -7,9 +7,14 @@ #include #include #include +#include +#include #include +#include +#include + #include BOOST_FIXTURE_TEST_SUITE(coinjoin_queue_tests, TestingSetup) @@ -96,4 +101,48 @@ BOOST_AUTO_TEST_CASE(queue_timestamp_validation) BOOST_CHECK(q.IsTimeOutOfBounds(current_time)); } +BOOST_AUTO_TEST_CASE(queue_timestamp_extreme_values) +{ + CCoinJoinQueue q; + q.nDenom = CoinJoin::AmountToDenomination(CoinJoin::GetSmallestDenomination()); + q.m_protxHash = uint256::ONE; + + // Negative timestamps are rejected by the guard + q.nTime = INT64_MIN; + BOOST_CHECK(q.IsTimeOutOfBounds(INT64_MAX)); + + q.nTime = INT64_MAX; + BOOST_CHECK(q.IsTimeOutOfBounds(INT64_MIN)); + + q.nTime = INT64_MIN; + BOOST_CHECK(q.IsTimeOutOfBounds(INT64_MIN)); + + // Large positive timestamp with same value: zero diff, in bounds + q.nTime = INT64_MAX; + BOOST_CHECK(!q.IsTimeOutOfBounds(INT64_MAX)); + + // Zero vs extreme positive: huge gap, out of bounds + q.nTime = 0; + BOOST_CHECK(q.IsTimeOutOfBounds(INT64_MAX)); + + // Zero vs negative: rejected by guard + q.nTime = 0; + BOOST_CHECK(q.IsTimeOutOfBounds(INT64_MIN)); +} + +static_assert(CoinJoin::CalculateAmountPriority(MAX_MONEY) == -(MAX_MONEY / COIN)); +static_assert(CoinJoin::CalculateAmountPriority(static_cast(INT64_MAX)) == 0); +static_assert(CoinJoin::CalculateAmountPriority(static_cast(-1)) == 0); + +BOOST_AUTO_TEST_CASE(calculate_amount_priority_guard) +{ + // Realistic amount: MAX_MONEY (21 million DASH) + BOOST_CHECK_EQUAL(CoinJoin::CalculateAmountPriority(MAX_MONEY), -(MAX_MONEY / COIN)); + + // Out-of-range amounts return 0 + BOOST_CHECK_EQUAL(CoinJoin::CalculateAmountPriority(static_cast(INT64_MAX)), 0); + BOOST_CHECK_EQUAL(CoinJoin::CalculateAmountPriority(static_cast(-1)), 0); + BOOST_CHECK_EQUAL(CoinJoin::CalculateAmountPriority(MAX_MONEY + 1), 0); +} + BOOST_AUTO_TEST_SUITE_END()