btcsignals: delete broken scoped_connection move assignment
What the developer wrote
Authored by Thomas
btcsignals: delete broken scoped_connection move assignment
The defaulted move assignment overwrites m_conn without disconnecting
it first, so the previous callback stays registered with the signal and
keeps firing, violating the RAII contract:
btcsignals::scoped_connection sc0 = sig.connect(IncrementCallback);
btcsignals::scoped_connection sc1 = sig.connect(SquareCallback);
sc0 = std::move(sc1);
val = 3; sig(val); // both callbacks fire: 16 instead of 9
Move assignment is unused in the codebase, so delete it rather than
fixing it. It can be implemented properly if a use case arises.
What changed, and why it matters
This commit removes a broken move-assignment operator from a small helper class that manages automatic disconnection of signal callbacks. The broken operator could leave an old callback registered after a move, causing unexpected behavior. The commit states the operator is unused in the codebase, so it is deleted rather than fixed. There is no direct evidence this bug was exploitable for security harm.
No urgent action needed; the change is a defensive cleanup. If backporting, ensure downstream code does not rely on the deleted move assignment. Consider adding a regression test if the operator is ever re-implemented.
Security signals we found
RAII contract violation in scoped resource management
Potential use-after-move-like semantic bug (stale callback remains registered)
No input validation or memory corruption signals present
No evidence of attacker-controlled trigger path
Evidence from the diff
The boost::signals2::scoped_connection wrapper in src/btcsignals.h had a defaulted move-assignment operator. That operator overwrote m_conn without first disconnecting the existing connection, so the previous callback remained registered and would continue firing. The patch deletes the move assignment (and updates the comment). The commit message explicitly says the operator is unused in the codebase, so the practical risk is low, but the bug violated RAII semantics and could cause logic errors if the operator were ever used.
Changed components
src/btcsignals.hboost::signals2::scoped_connection wrapperInspect captured patch +2 / −2
diff --git a/src/btcsignals.h b/src/btcsignals.h
index ebe7a5ac..9eeca769 100644
--- a/src/btcsignals.h
+++ b/src/btcsignals.h
@@ -123,11 +123,11 @@ public:
scoped_connection(connection rhs) noexcept : m_conn{std::move(rhs)} {}
scoped_connection(scoped_connection&&) noexcept = default;
- scoped_connection& operator=(scoped_connection&&) noexcept = default;
/**
- * For simplicity, disable copy assignment and construction.
+ * For simplicity, disable copy construction and copy/move assignment.
*/
+ scoped_connection& operator=(scoped_connection&&) = delete;
scoped_connection& operator=(const scoped_connection&) = delete;
scoped_connection(const scoped_connection&) = delete;
Why this scored 33/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.