Verify Electrum transaction responses before use
What the developer wrote
Authored by Elias Rohrer
Verify Electrum transaction responses before use
Electrum confirmations must reject transaction_get responses whose body
does not compute the requested txid. Otherwise a malicious server can
substitute an unrelated transaction and provide matching Merkle data for
the substituted body.
Co-Authored-By: HAL 9000
This finding was discovered by Project Loupe
What changed, and why it matters
This commit fixes a security hole in how a Bitcoin Lightning wallet talks to Electrum servers. Previously, the wallet asked a server for a specific transaction by its ID, but it did not check that the returned transaction actually matched that ID. A malicious or compromised Electrum server could send a different transaction instead, and the wallet would trust it. The fix adds a simple verification step: after receiving a transaction, the wallet recomputes its ID and rejects it if it does not match what was requested. The commit message explicitly says this prevents a malicious server from substituting an unrelated transaction and providing matching proof data.
Upgrade to a version containing this commit. If running an older version and using Electrum for transaction sync, treat Electrum servers as partially trusted and consider switching to a trusted Bitcoin Core RPC backend until patched. No immediate on-chain action is required for users already on a patched version.
Security signals we found
Commit title and message explicitly describe a security-relevant verification failure
Fix adds txid recomputation and equality check against requested txid
Fix is applied at multiple call sites where `transaction_get` results are consumed
Regression test added to prevent removal of the verification checks
Commit message attributes discovery to 'Project Loupe' and co-authorship to 'HAL 9000'
Evidence from the diff
In rust-lightning’s Electrum sync client, transaction_get(txid) responses were used without validating that tx.compute_txid() == txid. The patch adds this check at both call sites: one for watched transactions and one for transactions fetched while checking whether watched outputs are spent. If the computed txid does not equal the requested txid, the client now logs an error and returns InternalError::Failed. The patch also adds a regression test that asserts the source file contains the two verification checks. The commit message states that without this check, a malicious server can substitute an unrelated transaction and supply matching Merkle data for the substituted body.
Changed components
lightning-transaction-sync/src/electrum.rsElectrumSyncClient::sync_best_blockElectrumSyncClient::sync_best_block (watched output spend detection path)Inspect captured patch +30 / −0
diff --git a/lightning-transaction-sync/src/electrum.rs b/lightning-transaction-sync/src/electrum.rs
index 540cfc1..0283b9f 100644
--- a/lightning-transaction-sync/src/electrum.rs
+++ b/lightning-transaction-sync/src/electrum.rs
@@ -277,6 +277,11 @@ impl<L: Logger> ElectrumSyncClient<L> {
for txid in &sync_state.watched_transactions {
match self.client.transaction_get(&txid) {
Ok(tx) => {
+ if tx.compute_txid() != *txid {
+ log_error!(self.logger, "Retrieved transaction for txid {} doesn't match expectations. This should not happen. Please verify server integrity.", txid);
+ return Err(InternalError::Failed);
+ }
+
// Skip before using an arbitrary returned output to look up the
// transaction's script history.
if is_potentially_unsafe_merkle_leaf(&tx) {
@@ -365,6 +370,11 @@ impl<L: Logger> ElectrumSyncClient<L> {
match self.client.transaction_get(&txid) {
Ok(tx) => {
+ if tx.compute_txid() != txid {
+ log_error!(self.logger, "Retrieved transaction for txid {} doesn't match expectations. This should not happen. Please verify server integrity.", txid);
+ return Err(InternalError::Failed);
+ }
+
let mut is_spend = false;
for txin in &tx.input {
let watched_outpoint =
@@ -529,3 +539,23 @@ impl<L: Logger> Filter for ElectrumSyncClient<L> {
locked_queue.outputs.insert(output.outpoint.into_bitcoin_outpoint(), output);
}
}
+
+#[cfg(test)]
+mod tests {
+ #[test]
+ fn transaction_get_responses_are_verified_at_call_sites() {
+ let src = include_str!("electrum.rs");
+ let watched_transaction_check = concat!("if tx.compute_", "txid() != *txid");
+ let watched_output_spend_check = concat!("if tx.compute_", "txid() != txid");
+
+ assert!(
+ src.contains(watched_transaction_check),
+ "watched transaction_get responses must be verified against the requested txid"
+ );
+ assert!(
+ src.contains(watched_output_spend_check),
+ "watched-output spend transaction_get responses must be verified against the \
+ requested txid"
+ );
+ }
+}
Why this scored 74/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.