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

Author: tnull

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"
+		);
+	}
+}