Protect against Core's Merkle leaf node weakness

Bitcoin Core's Merkle tree implementation has no way to discern between
internal and leaf node entries. As a consequence it is susceptible to an
attacker injecting additional transactions by crafting 64-byte
transactions matching an inner Merkle node's hash (see
https://web.archive.org/web/20240329003521/https://bitslog.com/2018/06/09/leaf-node-weakness-in-bitcoin-merkle-tree-design/).

To protect against this (highly unlikely attack vector), we check that
the transaction isn't 64 bytes in length, and skip it otherwise.

diff --git a/lightning-transaction-sync/src/electrum.rs b/lightning-transaction-sync/src/electrum.rs
index e7edb3b7ee271b30d55339d458e51f0d91c618ae..ddd74743ff3eae371f5227498a44734b0a0716ac 100644 (file)
--- a/lightning-transaction-sync/src/electrum.rs
+++ b/lightning-transaction-sync/src/electrum.rs
@@ -270,6 +270,18 @@ where
                for txid in &sync_state.watched_transactions {
                        match self.client.transaction_get(&txid) {
                                Ok(tx) => {
+                                       // Bitcoin Core's Merkle tree implementation has no way to discern between
+                                       // internal and leaf node entries. As a consequence it is susceptible to an
+                                       // attacker injecting additional transactions by crafting 64-byte
+                                       // transactions matching an inner Merkle node's hash (see
+                                       // https://web.archive.org/web/20240329003521/https://bitslog.com/2018/06/09/leaf-node-weakness-in-bitcoin-merkle-tree-design/).
+                                       // To protect against this (highly unlikely) attack vector, we check that the
+                                       // transaction is at least 65 bytes in length.
+                                       if tx.total_size() == 64 {
+                                               log_error!(self.logger, "Skipping transaction {} due to retrieving potentially invalid tx data.", txid);
+                                               continue;
+                                       }
+
                                        watched_txs.push((txid, tx.clone()));
                                        if let Some(tx_out) = tx.output.first() {
                                                // We watch an arbitrary output of the transaction of interest in order to

diff --git a/lightning-transaction-sync/src/esplora.rs b/lightning-transaction-sync/src/esplora.rs
index f176d535124e67681cfabe7575e7eccad5edd0f4..b3e9285ec734986bdf7b1bb8fe0b4a4de22023e8 100644 (file)
--- a/lightning-transaction-sync/src/esplora.rs
+++ b/lightning-transaction-sync/src/esplora.rs
@@ -372,6 +372,22 @@ where
                                        return Err(InternalError::Failed);
                                }
 
+                               // Bitcoin Core's Merkle tree implementation has no way to discern between
+                               // internal and leaf node entries. As a consequence it is susceptible to an
+                               // attacker injecting additional transactions by crafting 64-byte
+                               // transactions matching an inner Merkle node's hash (see
+                               // https://web.archive.org/web/20240329003521/https://bitslog.com/2018/06/09/leaf-node-weakness-in-bitcoin-merkle-tree-design/).
+                               // To protect against this (highly unlikely) attack vector, we check that the
+                               // transaction is at least 65 bytes in length.
+                               if tx.total_size() == 64 {
+                                       log_error!(
+                                               self.logger,
+                                               "Skipping transaction {} due to retrieving potentially invalid tx data.",
+                                               txid
+                                       );
+                                       return Ok(None);
+                               }
+
                                if let Some(block_height) = known_block_height {
                                        // We can take a shortcut here if a previous call already gave us the height.
                                        return Ok(Some(ConfirmedTx { tx, txid, block_header, pos, block_height }));