Check correct merkle leaf size and validate txids on Electrum#4675
Conversation
|
👋 Thanks for assigning @wpaulino as a reviewer! |
SummaryI re-reviewed the current commit (1d13150) against my prior findings and verified the actual code state. Correction to a prior finding: My earlier "output-spend path lacks the merkle-leaf check" asymmetry finding (inline at electrum.rs:376) was a false positive. The New inline comment posted:
Still standing (minor, previously noted, not re-posted inline):
The core fix is correct: |
|
LGTM once the bot's comment is addressed |
tnull
force-pushed
the
2026-06-merkle-base-size
branch
from
57524e3 to
35e8dac
Compare
|
Updated to line-wrap commit messages. |
|
🔔 1st Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @wpaulino! This PR has been waiting for your review. |
Electrum confirmation checks must reject transactions whose non-witness serialization is 64 bytes, since txids and Merkle leaves are computed from that serialization. Witness padding can otherwise move total_size above 64 without removing the inner-node ambiguity. Co-Authored-By: HAL 9000 This finding was discovered by Project Loupe
Esplora confirmation checks must use the non-witness transaction size for the 64-byte Merkle leaf guard. Witness padding can otherwise raise total_size without changing the serialization hashed into the txid and Merkle tree. Co-Authored-By: HAL 9000 This finding was discovered by Project Loupe
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
tnull
force-pushed
the
2026-06-merkle-base-size
branch
from
35e8dac to
1d13150
Compare
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Thanks. Given these are just validation against a malicious server, which can certainly screw us in many other ways, is there a strong reason to want to backport this? Note that this does not resolve the issue of the merkle-tree-extension attack, where a valid transaction included in a block is 64 bytes and collides with a hash such that a proof can be forged for a different transaction. That is ultimately only solved by requesting a proof for the coinbase transaction in parallel, which I don't think is worth doing.
5 participants
Just a few minor Electrum/Esplora fixes.
These findings were discovered by Project Loupe.