verify fetched transactions match requested txid
What changed, and why it matters
This commit adds checks to make sure that when Sparrow Wallet asks an Electrum server for a specific Bitcoin transaction, the server actually returns the transaction that was requested. Before this change, a malicious or buggy server could return a different transaction than the one asked for, and Sparrow might trust it as if it were the right one. The fix verifies the transaction ID (a fingerprint of the transaction) matches in three places where transactions are fetched from the server.
Users should upgrade to a Sparrow Wallet release containing this commit. Server operators do not need to act. Developers should review whether any other network-facing deserialization paths trust server-declared identifiers without cryptographic verification.
Security signals we found
Missing input validation on server-provided transaction data
Potential transaction substitution by malicious or compromised Electrum server
Defense-in-depth verification added at data deserialization boundary
Exception handling improved to preserve error context
Evidence from the diff
The patch hardens Electrum server response handling by validating that returned transaction data hashes to the requested/declared txid. It adds checks in ElectrumServer.java for batched transaction fetches, verbose transaction fetches, and raw transaction fetches used to resolve output scripts. It also adds a defensive check in VerboseTransaction.getBlockTransaction() comparing the parsed transaction’s txid against the declared txid. A new unit test confirms the check accepts legacy and segwit transactions (which are declared by non-witness txid) and rejects mismatched hex. The change also improves exception propagation so verification failures are surfaced as ServerException with meaningful messages rather than swallowed causes.
Changed components
com.sparrowwallet.sparrow.net.ElectrumServercom.sparrowwallet.sparrow.net.VerboseTransactionElectrum server RPC transaction retrieval pathsInspect captured patch +100 / −16
diff --git a/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java b/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java
index 9c54213..180de19 100644
--- a/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java
+++ b/src/main/java/com/sparrowwallet/sparrow/net/ElectrumServer.java
@@ -865,6 +865,11 @@ public class ElectrumServer {
continue;
}
+ if(!transaction.getTxId().equals(hash)) {
+ log.error("Server returned transaction " + transaction.getTxId() + " for requested txid " + hash);
+ throw new IllegalStateException("Server returned a transaction that does not match the requested txid " + hash);
+ }
+
Optional<BlockTransactionHash> optionalReference = references.keySet().stream().filter(reference -> reference.getHash().equals(hash)).findFirst();
if(optionalReference.isEmpty()) {
throw new IllegalStateException("Returned transaction " + hash.toString() + " that was not requested");
@@ -913,7 +918,7 @@ public class ElectrumServer {
return transactionMap;
} catch (IllegalStateException e) {
- throw new ServerException(e.getCause());
+ throw new ServerException(e.getMessage(), e);
} catch (ElectrumServerRpcException e) {
throw new ServerException(e.getMessage(), e.getCause());
} catch (Exception e) {
@@ -1041,14 +1046,24 @@ public class ElectrumServer {
Map<String, VerboseTransaction> result = electrumServerRpc.getVerboseTransactions(getTransport(), txids, scriptHash);
- Map<Sha256Hash, BlockTransaction> transactionMap = new HashMap<>();
- for(String txid : result.keySet()) {
- Sha256Hash hash = Sha256Hash.wrap(txid);
- BlockTransaction blockTransaction = result.get(txid).getBlockTransaction();
- transactionMap.put(hash, blockTransaction);
- }
+ try {
+ Map<Sha256Hash, BlockTransaction> transactionMap = new HashMap<>();
+ for(String txid : result.keySet()) {
+ Sha256Hash hash = Sha256Hash.wrap(txid);
+ VerboseTransaction verboseTransaction = result.get(txid);
+ if(!hash.equals(Sha256Hash.wrap(verboseTransaction.txid))) {
+ log.error("Server returned transaction " + verboseTransaction.txid + " for requested txid " + hash);
+ throw new ServerException("Server returned a transaction that does not match the requested txid " + hash);
+ }
+
+ transactionMap.put(hash, verboseTransaction.getBlockTransaction());
+ }
- return transactionMap;
+ return transactionMap;
+ } catch(RuntimeException e) {
+ log.error("Could not retrieve referenced transactions", e);
+ throw new ServerException("Could not retrieve referenced transactions", e);
+ }
}
public Map<Integer, Double> getFeeEstimates(List<Integer> targetBlocks, boolean useCached) throws ServerException {
@@ -1307,17 +1322,26 @@ public class ElectrumServer {
continue;
}
+ Transaction transaction;
+
try {
- Transaction transaction = new Transaction(Utils.hexToBytes(strRawTx));
- for(TransactionOutput txOutput : transaction.getOutputs()) {
- if(txOutput.getScript().equals(outputScript)) {
- transactionOutputs.add(txOutput);
- }
- }
- transactions.add(transaction);
+ transaction = new Transaction(Utils.hexToBytes(strRawTx));
} catch(ProtocolException e) {
log.error("Could not parse tx: " + strRawTx);
+ continue;
+ }
+
+ if(!transaction.getTxId().toString().equalsIgnoreCase(txid)) {
+ log.error("Server returned transaction " + transaction.getTxId() + " for requested txid " + txid);
+ throw new ServerException("Server returned a transaction that does not match the requested txid " + txid);
+ }
+
+ for(TransactionOutput txOutput : transaction.getOutputs()) {
+ if(txOutput.getScript().equals(outputScript)) {
+ transactionOutputs.add(txOutput);
+ }
}
+ transactions.add(transaction);
}
for(Transaction transaction : transactions) {
diff --git a/src/main/java/com/sparrowwallet/sparrow/net/VerboseTransaction.java b/src/main/java/com/sparrowwallet/sparrow/net/VerboseTransaction.java
index bedfc12..527d94f 100644
--- a/src/main/java/com/sparrowwallet/sparrow/net/VerboseTransaction.java
+++ b/src/main/java/com/sparrowwallet/sparrow/net/VerboseTransaction.java
@@ -44,6 +44,12 @@ public class VerboseTransaction {
}
public BlockTransaction getBlockTransaction() {
- return new BlockTransaction(Sha256Hash.wrap(txid), getHeight(), getDate(), 0L, new Transaction(Utils.hexToBytes(hex)), blockhash == null ? null : Sha256Hash.wrap(blockhash));
+ Sha256Hash declaredTxid = Sha256Hash.wrap(txid);
+ Transaction transaction = new Transaction(Utils.hexToBytes(hex));
+ if(!transaction.getTxId().equals(declaredTxid)) {
+ throw new IllegalStateException("Server returned transaction " + transaction.getTxId() + " for declared txid " + declaredTxid);
+ }
+
+ return new BlockTransaction(declaredTxid, getHeight(), getDate(), 0L, transaction, blockhash == null ? null : Sha256Hash.wrap(blockhash));
}
}
diff --git a/src/test/java/com/sparrowwallet/sparrow/net/VerboseTransactionTest.java b/src/test/java/com/sparrowwallet/sparrow/net/VerboseTransactionTest.java
new file mode 100644
index 0000000..004e12a
--- /dev/null
+++ b/src/test/java/com/sparrowwallet/sparrow/net/VerboseTransactionTest.java
@@ -0,0 +1,54 @@
+package com.sparrowwallet.sparrow.net;
+
+import com.sparrowwallet.drongo.wallet.BlockTransaction;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class VerboseTransactionTest {
+ private static final String LEGACY_TXID = "b78d4708ed7e9d8f023231c770b35727e23689b28a09eae017965a38e41dec83";
+ private static final String LEGACY_HEX = "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0804e6ed5b1b02b000ffffffff0100f2052a01000000434104ab3779ba979cd2f7d76fd1b6a57f42bf4bdd9210409a693a46d6d426c0ba021aca2f364ce5141b7721b47fb5f34ce7301abbab24c067048b721c633ae65e1af0ac00000000";
+
+ private static final String SEGWIT_TXID = "f8fc281eb915a1b350f89ffc5090c71dc65acaf6e59e29dc5afe53d842f19d01";
+ private static final String SEGWIT_WTXID = "ef6da0d3f41a666c0eb914cf592caa803aa465c25cf630f786cd3384619605ad";
+ private static final String SEGWIT_HEX = "020000000001014596cc6219630c13cbca099838c2fb0920cde29de1e5473087de8bbce06b9f510100000000ffffffff02502a4b000000000017a914b1cd708c9d49c7ad6ec851ad7f24076233fa7cfb8772915600000000001600145279dddc177883923bcf3bd5aab50e725dca01f302483045022100e9056474685b7d885956c7c7e5ac77e1249373e5d222b13620dcde6a63e337d602206ebb59c1834e991e9c9f6129a78c7669cfc4c41c4d19c6be4dabe6749715d5ee01210278f5f957591a07a51fc5033c3407de2ff722b0a5f98e91c9a9e1e038c9b1b59300000000";
+
+ @Test
+ public void acceptsHexMatchingDeclaredTxid() {
+ BlockTransaction blockTransaction = verboseTransaction(LEGACY_TXID, LEGACY_HEX).getBlockTransaction();
+
+ assertEquals(LEGACY_TXID, blockTransaction.getHashAsString());
+ assertEquals(blockTransaction.getHash(), blockTransaction.getTransaction().getTxId());
+ }
+
+ /**
+ * Segwit transactions are requested and declared by their non-witness txid, but are serialized with their witness data.
+ * Comparing the wtxid here would reject every segwit transaction.
+ */
+ @Test
+ public void acceptsSegwitTransactionDeclaredByTxid() {
+ BlockTransaction blockTransaction = verboseTransaction(SEGWIT_TXID, SEGWIT_HEX).getBlockTransaction();
+
+ assertEquals(SEGWIT_TXID, blockTransaction.getHashAsString());
+ assertEquals(SEGWIT_WTXID, blockTransaction.getTransaction().getWTxId().toString());
+ assertNotEquals(blockTransaction.getTransaction().getTxId(), blockTransaction.getTransaction().getWTxId());
+ }
+
+ @Test
+ public void rejectsHexNotMatchingDeclaredTxid() {
+ VerboseTransaction verboseTransaction = verboseTransaction(LEGACY_TXID, SEGWIT_HEX);
+
+ assertThrows(IllegalStateException.class, verboseTransaction::getBlockTransaction);
+ }
+
+ private VerboseTransaction verboseTransaction(String txid, String hex) {
+ VerboseTransaction verboseTransaction = new VerboseTransaction();
+ verboseTransaction.txid = txid;
+ verboseTransaction.hex = hex;
+ verboseTransaction.confirmations = 0;
+
+ return verboseTransaction;
+ }
+}
Why this scored 72/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.