rpc: Fix for duplicate external signers case
What the developer wrote
Authored by optout
rpc: Fix for duplicate external signers case
In case of multiple external signers, with some duplicates,
de-duplicate them: keep one signer per fingerprint, and
keep all non-duplicates as well. Add a new test check.
What changed, and why it matters
This commit fixes a bug in how Bitcoin Core lists external hardware wallets (signers). Previously, if the list contained duplicate signers, the code would stop processing entirely after the first duplicate, potentially hiding valid non-duplicate signers from the user. The fix changes the behavior so duplicates are skipped while the rest of the list is still processed. This is a correctness and usability fix rather than a direct theft-of-funds vulnerability, but it could mislead a user about which hardware wallets are available.
Treat as a low-severity bug fix. No urgent security response is warranted, but the fix should be included in normal release maintenance. Users relying on multiple external signers should upgrade to avoid incomplete signer lists.
Security signals we found
Logic error causing early termination of enumeration loop
Potential UI/API misrepresentation of available signers
No authentication, cryptographic, or memory-safety flaw evident
Evidence from the diff
In ExternalSigner::Enumerate, the loop over signer entries used break when a duplicate fingerprint was found. This caused enumeration to terminate early, dropping any subsequent signers (including non-duplicates). The patch changes break to continue, so only the duplicate entry is skipped and remaining entries are still evaluated. A functional test is added to verify that duplicates are de-duplicated while non-duplicates remain present.
Changed components
src/external_signer.cppRPC enumeratesignersExternal signer enumerationInspect captured patch +14 / −1
diff --git a/src/external_signer.cpp b/src/external_signer.cpp
index 2da95026..da75de77 100644
--- a/src/external_signer.cpp
+++ b/src/external_signer.cpp
@@ -53,7 +53,7 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector<ExternalS
for (const ExternalSigner& signer : signers) {
if (signer.m_fingerprint.compare(fingerprintStr) == 0) duplicate = true;
}
- if (duplicate) break;
+ if (duplicate) continue;
std::string name;
const UniValue& model_field = signer.find_value("model");
if (model_field.isStr() && model_field.getValStr() != "") {
diff --git a/test/functional/rpc_signer.py b/test/functional/rpc_signer.py
index 3b1fe88d..51a01249 100755
--- a/test/functional/rpc_signer.py
+++ b/test/functional/rpc_signer.py
@@ -72,6 +72,19 @@ class RPCSignerTest(BitcoinTestFramework):
)
self.clear_mock_result(self.nodes[1])
+ # Duplicate fingerprints
+ self.set_mock_result(self.nodes[1],
+ '0 ['
+ '{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"}, '
+ '{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"}, '
+ '{"fingerprint": "00000002", "type": "trezor", "model": "trezor_one"}'
+ ']')
+ assert_equal(self.nodes[1].enumeratesigners(), {"signers": [
+ {"fingerprint": "00000001", "name": "trezor_t"},
+ {"fingerprint": "00000002", "name": "trezor_one"},
+ ]})
+ self.clear_mock_result(self.nodes[1])
+
assert_equal({'fingerprint': '00000001', 'name': 'trezor_t'} in self.nodes[1].enumeratesigners()['signers'], True)
if __name__ == '__main__':
Why this scored 25/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.