Skip to content

Commit 56eec70

Browse files
MarcoFalkeapoelstra
MarcoFalke
authored andcommitted
Fix signed integer overflow in prioritisetransaction RPC
Cherry-pick of fa52cf8 bitcoin/bitcoin#23418 (2/2)
1 parent 6043ab4 commit 56eec70

File tree

2 files changed

+9
-8
lines changed

2 files changed

+9
-8
lines changed

src/txmempool.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <reverse_iterator.h>
2121
#include <util/check.h>
2222
#include <util/moneystr.h>
23+
#include <util/overflow.h>
2324
#include <util/system.h>
2425
#include <util/time.h>
2526
#include <validationinterface.h>
@@ -102,9 +103,9 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee,
102103

103104
void CTxMemPoolEntry::UpdateModifiedFee(CAmount fee_diff)
104105
{
105-
nModFeesWithDescendants += fee_diff;
106-
nModFeesWithAncestors += fee_diff;
107-
m_modified_fee += fee_diff;
106+
nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, fee_diff);
107+
nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, fee_diff);
108+
m_modified_fee = SaturatingAdd(m_modified_fee, fee_diff);
108109
}
109110

110111
void CTxMemPoolEntry::UpdateLockPoints(const LockPoints& lp)
@@ -459,7 +460,7 @@ void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFe
459460
{
460461
nSizeWithDescendants += modifySize;
461462
assert(int64_t(nSizeWithDescendants) > 0);
462-
nModFeesWithDescendants += modifyFee;
463+
nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, modifyFee);
463464
nCountWithDescendants += modifyCount;
464465
assert(int64_t(nCountWithDescendants) > 0);
465466
}
@@ -468,7 +469,7 @@ void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee,
468469
{
469470
nSizeWithAncestors += modifySize;
470471
assert(int64_t(nSizeWithAncestors) > 0);
471-
nModFeesWithAncestors += modifyFee;
472+
nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, modifyFee);
472473
nCountWithAncestors += modifyCount;
473474
assert(int64_t(nCountWithAncestors) > 0);
474475
nSigOpCostWithAncestors += modifySigOps;
@@ -1018,7 +1019,7 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD
10181019
{
10191020
LOCK(cs);
10201021
CAmount &delta = mapDeltas[hash];
1021-
delta += nFeeDelta;
1022+
delta = SaturatingAdd(delta, nFeeDelta);
10221023
txiter it = mapTx.find(hash);
10231024
if (it != mapTx.end()) {
10241025
mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); });

test/sanitizer_suppressions/ubsan

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
# -fsanitize=undefined suppressions
22
# =================================
3-
# This would be `signed-integer-overflow:CTxMemPool::PrioritiseTransaction`,
3+
# The suppressions would be `sanitize-type:ClassName::MethodName`,
44
# however due to a bug in clang the symbolizer is disabled and thus no symbol
55
# names can be used.
66
# See https://github.com/google/sanitizers/issues/1364
7-
signed-integer-overflow:txmempool.cpp
7+
88
# https://github.com/bitcoin/bitcoin/pull/21798#issuecomment-829180719
99
signed-integer-overflow:policy/feerate.cpp
1010

0 commit comments

Comments
 (0)