external_signer: validate fingerprint from enumerate response #35639

pull kwsantiago wants to merge 1 commits into bitcoin:master from privkeyio:signer-validate-fingerprint changing 2 files +9 −0

kwsantiago commented at 2:24 AM on July 2, 2026: contributor

enumeratesigners takes the fingerprint field from the external signer's enumerate output and stores it without checking it. That value is later handed back to the signer command as --fingerprint <value> (e.g. in displayaddress), so a malformed value propagates unchecked.

A master key fingerprint is 4 bytes, i.e. 8 hex characters. This adds a check that the reported fingerprint is exactly 8 hex characters and throws a clear error otherwise. A functional test covers empty, wrong-length, and non-hex inputs.
DrahtBot commented at 2:24 AM on July 2, 2026: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35639.



Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK Sjors, sedited, achow101

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
in src/external_signer.cpp:51 in 7b82478eee outdated
```
  47 | @@ -48,6 +48,9 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector<ExternalS
  48 |              throw std::runtime_error(strprintf("'%s' received invalid response, missing signer fingerprint", command));
  49 |          }
  50 |          const std::string& fingerprintStr{fingerprint.get_str()};
  51 | +        if (fingerprintStr.size() != 8 || !IsHex(fingerprintStr)) {
```
vicjuma commented at 12:29 PM on July 2, 2026:

Concept-wise this makes sense, but is it addressing a specific case you've observed — i.e. a supported external signer actually returning a malformed fingerprint? A linked issue or example would help clarify the failure and provide a useful context.

kwsantiago commented at 1:23 PM on July 2, 2026:

No specific issue I ran into specifically which I can report. A master fingerprint is always 8 hex chars, so this just validates it at the boundary and fails with a clear error instead of letting a bad value reach ParseHex() in SignTransaction
external_signer: validate fingerprint from enumerate response 4c9de7d5b3
kwsantiago force-pushed on Jul 2, 2026
Sjors commented at 4:52 PM on July 4, 2026: member

utACK 4c9de7d5b329f8ffca7222d8f53c2f5a5616dfc9

The docs already specify that fingerprints must be 8 hex characters: https://github.com/bitcoin/bitcoin/blob/master/doc/external-signer.md#flag---fingerprint-fingerprint-required-except-for-enumerate
willcl-ark added the label Wallet on Jul 8, 2026
sedited commented at 3:27 PM on July 10, 2026: contributor

@kwsantiago can you elaborate on how you found this and if you ran into this error somewhere?
kwsantiago commented at 4:08 PM on July 10, 2026: contributor

@sedited No, I didn't hit it at runtime, it came from reviewing the enumerate response handling, where the fingerprint is the one field taken from the signer and used unchecked. I went into the concrete path (ParseHex in SignTransaction) here: #35639 (review)
sedited approved
sedited commented at 4:54 PM on July 10, 2026: contributor

ACK 4c9de7d5b329f8ffca7222d8f53c2f5a5616dfc9
achow101 commented at 9:05 PM on July 14, 2026: member

ACK 4c9de7d5b329f8ffca7222d8f53c2f5a5616dfc9
achow101 merged this on Jul 14, 2026
achow101 closed this on Jul 14, 2026
kwsantiago deleted the branch on Jul 14, 2026
Kino1994 referenced this in commit dcb43478ac on Jul 19, 2026
sedited referenced this in commit 84c89f1baa on Jul 28, 2026
Kino1994 referenced this in commit 12eadef2ea on Aug 19, 2026
alexanderwiederin referenced this in commit e9bfecbbed on Aug 26, 2026

Type	Reviewers
ACK	Sjors, sedited, achow101

Contributors

Labels

external_signer: validate fingerprint from enumerate response #35639

Code Coverage & Benchmarks

Reviews