kernel: guard btck::Handle move-assignment against self-move
What the developer wrote
Authored by Thomas
kernel: guard btck::Handle move-assignment against self-move
The move-assignment operator for btck::Handle<> unconditionally called
DestroyFunc(m_ptr) before reading the source pointer. On a self-move
(h = std::move(h)), this destroyed the held resource and then reassigned
the now-dangling pointer back to m_ptr via std::exchange, leading to a
double-free when the object is later destroyed.
Mirror the existing self-check in the copy-assignment operator by
guarding the move-assignment with 'if (this != &other)' so a self-move
becomes a no-op, leaving the object in a valid state as required by the
standard library.
Handle<> is the base of 16 public types in the kernel C++ API wrapper
(Transaction, Block, BlockHeader, ChainParams, Context, Coin,
BlockValidationState, ScriptPubkey, TransactionOutput, Txid, OutPoint,
TransactionInput, PrecomputedTransactionData, BlockHash,
BlockSpentOutputs, TransactionSpentOutputs), so self-move can arise
from generic algorithms operating on containers of these types.
Extend CheckHandle in test_kernel to cover self-move-assignment for
every Handle-derived type.
What changed, and why it matters
This commit fixes a bug in Bitcoin Core's kernel C++ API wrapper where a special kind of assignment—moving an object into itself—could accidentally destroy its own underlying resource and later cause a crash (double-free) when the object is cleaned up. The fix adds a self-check, similar to one already used for copy assignment, so self-moves do nothing harmful. The change also adds tests for all 16 public types built on this wrapper.
Apply the patch. The fix is small, well-scoped, and includes regression tests. Review any code using these kernel API types with containers or algorithms that may perform moves (e.g., sorting, vector reallocation) to ensure the fix is present.
Security signals we found
Double-free/use-after-free in resource-managing wrapper
Self-move-assignment not guarded
Affects 16 public kernel API types
Could be triggered by generic container/algorithms
Existing copy-assignment had self-check but move-assignment did not
Evidence from the diff
The move-assignment operator of btck::Handle<> in src/kernel/bitcoinkernel_wrapper.h previously called DestroyFunc(m_ptr) before reading other.m_ptr. On self-move (h = std::move(h)), this destroyed the held resource, then std::exchange wrote the now-dangling pointer back into m_ptr, producing a use-after-free/double-free on later destruction. The patch guards the move-assignment with if (this != &other), making self-move a no-op. Tests in src/test/kernel/test_kernel.cpp are extended to exercise self-move-assignment for every Handle<>-derived type.
Changed components
src/kernel/bitcoinkernel_wrapper.hsrc/test/kernel/test_kernel.cppbtck::Handle<> templateHandle-derived public types: Transaction, Block, BlockHeader, ChainParams, Context, Coin, BlockValidationState, ScriptPubkey, Txid, OutPoint, TransactionInput, PrecomputedTransactionData, BlockHash, BlockSpentOutputs, TransactionSpentOutputsInspect captured patch +14 / −2
diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h
index 38dd709f..12299dfa 100644
--- a/src/kernel/bitcoinkernel_wrapper.h
+++ b/src/kernel/bitcoinkernel_wrapper.h
@@ -341,8 +341,10 @@ public:
Handle(Handle&& other) noexcept : m_ptr(other.m_ptr) { other.m_ptr = nullptr; }
Handle& operator=(Handle&& other) noexcept
{
- DestroyFunc(m_ptr);
- m_ptr = std::exchange(other.m_ptr, nullptr);
+ if (this != &other) {
+ DestroyFunc(m_ptr);
+ m_ptr = std::exchange(other.m_ptr, nullptr);
+ }
return *this;
}
diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
index b22a5ede..76b8398a 100644
--- a/src/test/kernel/test_kernel.cpp
+++ b/src/test/kernel/test_kernel.cpp
@@ -315,6 +315,16 @@ void CheckHandle(T object, T distinct_object)
if constexpr (HasToBytes<T>) {
check_equal(object2.ToBytes(), object3.ToBytes());
}
+
+ // Self move-assignment must not destroy the held resource.
+ // Use a reference to avoid -Wself-move warnings.
+ original_ptr = object2.get();
+ auto& object2_ref = object2;
+ object2 = std::move(object2_ref);
+ BOOST_CHECK_EQUAL(object2.get(), original_ptr);
+ if constexpr (HasToBytes<T>) {
+ check_equal(object2.ToBytes(), object3.ToBytes());
+ }
}
template <typename RangeType>
Why this scored 62/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.