Skip to content

Commit e0919b2

Browse files
aydencook03aydencook03
authored
Merge branch 'spesmilo:master' into watchonly-key-origin
2 parents 51f6a25 + 294d214 commit e0919b2

4 files changed

Lines changed: 110 additions & 40 deletions

File tree

‎contrib/ci/claude_security_review.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535

3636
MAX_DIFF_CHARS = 800_000
3737
CLAUDE_TIMEOUT_SECONDS = 20 * 60
38-
CLAUDE_MODEL = "claude-opus-4-6"
38+
CLAUDE_MODEL = "claude-opus-4-7"
3939
CLAUDE_EFFORT = "max"
4040

4141
VERDICT_PASS = "PASS"

‎electrum/verifier.py‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class MerkleVerificationFailure(Exception): pass
4343
class MissingBlockHeader(MerkleVerificationFailure): pass
4444
class MerkleRootMismatch(MerkleVerificationFailure): pass
4545
class InnerNodeOfSpvProofIsValidTx(MerkleVerificationFailure): pass
46+
class LeftSiblingDuplicate(MerkleVerificationFailure): pass
4647

4748

4849
class SPV(NetworkJobOnDefaultServer):
@@ -149,11 +150,16 @@ def hash_merkle_root(cls, merkle_branch: Sequence[str], tx_hash: str, leaf_pos_i
149150
if leaf_pos_in_tree < 0:
150151
raise MerkleVerificationFailure('leaf_pos_in_tree must be non-negative')
151152
index = leaf_pos_in_tree
152-
for item in merkle_branch_bytes:
153-
if len(item) != 32:
153+
for sibling in merkle_branch_bytes:
154+
if len(sibling) != 32:
154155
raise MerkleVerificationFailure('all merkle branch items have to be 32 bytes long')
155-
inner_node = (item + h) if (index & 1) else (h + item)
156+
is_right_child = (index & 1)
157+
inner_node = (sibling + h) if is_right_child else (h + sibling)
158+
# CVE-2017-12842 protection: inner node must not be a valid tx
156159
cls._raise_if_valid_tx(inner_node.hex())
160+
# CVE-2012-2459 protection: reject left-sibling duplicates
161+
if is_right_child and sibling == h:
162+
raise LeftSiblingDuplicate()
157163
h = sha256d(inner_node)
158164
index >>= 1
159165
if index != 0:
@@ -162,7 +168,7 @@ def hash_merkle_root(cls, merkle_branch: Sequence[str], tx_hash: str, leaf_pos_i
162168

163169
@classmethod
164170
def _raise_if_valid_tx(cls, raw_tx: str):
165-
# If an inner node of the merkle proof is also a valid tx, chances are, this is an attack.
171+
# CVE-2017-12842: If an inner node of the merkle proof is also a valid tx, chances are, this is an attack.
166172
# https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-June/016105.html
167173
# https://lists.linuxfoundation.org/pipermail/bitcoin-dev/attachments/20180609/9f4f5b1f/attachment-0001.pdf
168174
# https://bitcoin.stackexchange.com/questions/76121/how-is-the-leaf-node-weakness-in-merkle-trees-exploitable/76122#76122

‎tests/test_lnpeer.py‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1883,25 +1883,15 @@ async def run_test(test_trampoline):
18831883
del bob_w._preimages[pay_req.rhash] # del preimage so bob doesn't settle
18841884
payment_key = bob_w._get_payment_key(lnaddr.paymenthash).hex()
18851885

1886-
cb_got_called = False
1886+
cb_got_called = asyncio.Event()
18871887
async def cb(_payment_hash):
18881888
self.logger.debug(f"hold invoice callback called. {bob_w.network.get_local_height()=}")
1889-
nonlocal cb_got_called
1890-
cb_got_called = True
1889+
cb_got_called.set()
18911890

18921891
bob_w.register_hold_invoice(lnaddr.paymenthash, cb)
18931892

18941893
async def check_mpp_state():
1895-
async def wait_for_resolution():
1896-
while True:
1897-
await asyncio.sleep(0.1)
1898-
if payment_key not in bob_w.received_mpp_htlcs:
1899-
continue
1900-
if not bob_w.received_mpp_htlcs[payment_key].resolution == RecvMPPResolution.SETTLING:
1901-
continue
1902-
return
1903-
await util.wait_for2(wait_for_resolution(), timeout=2)
1904-
assert cb_got_called
1894+
await util.wait_for2(cb_got_called.wait(), timeout=2)
19051895
mpp_set = bob_w.received_mpp_htlcs[payment_key]
19061896
self.assertEqual(mpp_set.resolution, RecvMPPResolution.SETTLING, msg=mpp_set.resolution)
19071897
self.assertEqual(len(mpp_set.htlcs), 1, f"should get only one htlc: {mpp_set.htlcs=}")

‎tests/test_verifier.py‎

Lines changed: 96 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,49 +3,123 @@
33
from electrum.bitcoin import hash_encode
44
from electrum.transaction import Transaction
55
from electrum.util import bfh
6-
from electrum.verifier import SPV, InnerNodeOfSpvProofIsValidTx
6+
from electrum.verifier import SPV, InnerNodeOfSpvProofIsValidTx, LeftSiblingDuplicate
77

88
from . import ElectrumTestCase
99

1010

11-
MERKLE_BRANCH = [
12-
'f2994fd4546086b21b4916b76cf901afb5c4db1c3ecbfc91d6f4cae1186dfe12',
13-
'6b65935528311901c7acda7db817bd6e3ce2f05d1c62c385b7caadb65fac7520']
14-
15-
MERKLE_ROOT = '11dbac015b6969ea75509dd1250f33c04ec4d562c2d895de139a65f62f808254'
16-
17-
VALID_64_BYTE_TX = ('0200000001cb659c5528311901a7aada7db817bd6e3ce2f05d1c62c385b7caad'
18-
'b65fac75201234000000fabcdefa01abcd1234010000000405060708fabcdefa')
19-
assert len(VALID_64_BYTE_TX) == 128
20-
21-
22-
class VerifierTestCase(ElectrumTestCase):
23-
# these tests are regarding the attack described in
11+
class TestVerifier_CVE_2017_12842(ElectrumTestCase):
12+
# these tests are regarding CVE-2017-12842, the attack described in
2413
# https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-June/016105.html
2514
TESTNET = True
2615

16+
MERKLE_BRANCH = [
17+
'f2994fd4546086b21b4916b76cf901afb5c4db1c3ecbfc91d6f4cae1186dfe12',
18+
'6b65935528311901c7acda7db817bd6e3ce2f05d1c62c385b7caadb65fac7520',
19+
]
20+
MERKLE_ROOT = '11dbac015b6969ea75509dd1250f33c04ec4d562c2d895de139a65f62f808254'
21+
VALID_64_BYTE_TX = ('0200000001cb659c5528311901a7aada7db817bd6e3ce2f05d1c62c385b7caad'
22+
'b65fac75201234000000fabcdefa01abcd1234010000000405060708fabcdefa')
23+
assert len(VALID_64_BYTE_TX) == 128
24+
2725
def test_verify_ok_t_tx(self):
2826
"""Actually mined 64 byte tx should not raise."""
29-
t_tx = Transaction(VALID_64_BYTE_TX)
27+
t_tx = Transaction(self.VALID_64_BYTE_TX)
3028
t_tx_hash = t_tx.txid()
31-
self.assertEqual(MERKLE_ROOT, SPV.hash_merkle_root(MERKLE_BRANCH, t_tx_hash, 3))
29+
self.assertEqual(self.MERKLE_ROOT, SPV.hash_merkle_root(self.MERKLE_BRANCH, t_tx_hash, 3))
3230

3331
def test_verify_fail_f_tx_odd(self):
3432
"""Raise if inner node of merkle branch is valid tx. ('odd' fake leaf position)"""
3533
# first 32 bytes of T encoded as hash
36-
fake_branch_node = hash_encode(bfh(VALID_64_BYTE_TX[:64]))
37-
fake_mbranch = [fake_branch_node] + MERKLE_BRANCH
34+
fake_branch_node = hash_encode(bfh(self.VALID_64_BYTE_TX[:64]))
35+
fake_mbranch = [fake_branch_node] + self.MERKLE_BRANCH
3836
# last 32 bytes of T encoded as hash
39-
f_tx_hash = hash_encode(bfh(VALID_64_BYTE_TX[64:]))
37+
f_tx_hash = hash_encode(bfh(self.VALID_64_BYTE_TX[64:]))
4038
with self.assertRaises(InnerNodeOfSpvProofIsValidTx):
4139
SPV.hash_merkle_root(fake_mbranch, f_tx_hash, 7)
4240

4341
def test_verify_fail_f_tx_even(self):
4442
"""Raise if inner node of merkle branch is valid tx. ('even' fake leaf position)"""
4543
# last 32 bytes of T encoded as hash
46-
fake_branch_node = hash_encode(bfh(VALID_64_BYTE_TX[64:]))
47-
fake_mbranch = [fake_branch_node] + MERKLE_BRANCH
44+
fake_branch_node = hash_encode(bfh(self.VALID_64_BYTE_TX[64:]))
45+
fake_mbranch = [fake_branch_node] + self.MERKLE_BRANCH
4846
# first 32 bytes of T encoded as hash
49-
f_tx_hash = hash_encode(bfh(VALID_64_BYTE_TX[:64]))
47+
f_tx_hash = hash_encode(bfh(self.VALID_64_BYTE_TX[:64]))
5048
with self.assertRaises(InnerNodeOfSpvProofIsValidTx):
5149
SPV.hash_merkle_root(fake_mbranch, f_tx_hash, 6)
50+
51+
52+
class TestVerifier_CVE_2012_2459(ElectrumTestCase):
53+
# These tests are regarding CVE-2012-2459.
54+
# Bitcoin's Merkle tree duplicates odd nodes to balance the tree. An attacker can
55+
# exploit this by constructing a tree where a duplicated subtree is treated
56+
# as containing real leaves, allowing forged proofs for phantom leaf positions.
57+
#
58+
# Example with 11 real leaves and forged 16-leaf claim:
59+
#
60+
# Real tree (11 leaves):
61+
#
62+
# **root**
63+
# __________/ \_________
64+
# / \
65+
# 14 c Height 3
66+
# _ / \ _ / \
67+
# / \ / \
68+
# 6 13 b b' Height 2
69+
# / \ / \ / \
70+
# 2 5 9 12 17 a Height 1
71+
# / \ / \ / \ / \ / \ / \
72+
# 0 1 3 4 7 8 10 11 15 16 18 18' Height 0
73+
# --------------------------------------------------------
74+
# 0 1 2 3 4 5 6 7 8 9 10 Leaf index
75+
#
76+
# Nodes marked with ' are duplicates to balance the tree.
77+
#
78+
# Forged tree (attacker claims 16 leaves):
79+
#
80+
# **root**
81+
# __________/ \________________
82+
# / \
83+
# 14 c Height 3
84+
# _ / \ _ _____/ \_____
85+
# / \ / \
86+
# 6 13 b b' Height 2
87+
# / \ / \ / \ / \
88+
# 2 5 9 12 17 a 17' a' Height 1
89+
# / \ / \ / \ / \ / \ / \ / \ / \
90+
# 0 1 3 4 7 8 10 11 15 16 18 18' 15' 16' 18' 18' Height 0
91+
# --------------------------------------------------------------------------
92+
# 0 1 2 3 4 5 6 7 8 9 10 11! 12! 13! 14! 15! Leaf index
93+
#
94+
# Nodes with ! are phantom leaves. The attacker duplicated the entire
95+
# subtree under 'b' to create fake leaves 11-15.
96+
#
97+
# The attack works because:
98+
# - Real proof for leaf 10: [18', 17 , b', 14] with b' as RIGHT sibling
99+
# - Forged proof for leaf 14: [18', 17', b , 14] with b as LEFT sibling
100+
#
101+
# We can guard against this: in forged proofs, a duplicate will appear as a LEFT sibling
102+
# (sibling == current when index bit is 1).
103+
# Legitimate duplicates for balancing only appear as RIGHT siblings.
104+
TESTNET = True
105+
106+
# from testnet3 block 4909055 (https://blockstream.info/testnet/block/00000000c4a54b073c224bbf1f7c40cc85498a823e1dd5d20be51e6464a3dab9)
107+
# but even if it gets reorged, point is:
108+
# - block has 3 txns total, so valid indices [0,1,2]
109+
# - next power of 2 is 4, so merkle tree leaves will be [t0,t1,t2,t2']
110+
# - TXID is for t2, so its real index is 2, but index 3 could be "forged" for it as well
111+
MERKLE_BRANCH = [
112+
'9b2c7e407188465594832cfbe84c9758029084527c855ea29a16603e5d1c51b6',
113+
'a8484ccbaa74ffa060d0a500f7ce3ea4953beace18df8384024dfa9290385b1c',
114+
]
115+
MERKLE_ROOT = '3465af659f6438b133c6d980accbb61b7be43f8ad899e40054e33b37aecba28e'
116+
TXID = '9b2c7e407188465594832cfbe84c9758029084527c855ea29a16603e5d1c51b6'
117+
118+
def test_valid_right_sibling_duplicate(self):
119+
leaf_pos_in_tree = 2
120+
self.assertEqual(self.MERKLE_ROOT, SPV.hash_merkle_root(self.MERKLE_BRANCH, self.TXID, leaf_pos_in_tree))
121+
122+
def test_malicious_left_sibling_duplicate(self):
123+
leaf_pos_in_tree = 3
124+
with self.assertRaises(LeftSiblingDuplicate):
125+
SPV.hash_merkle_root(self.MERKLE_BRANCH, self.TXID, leaf_pos_in_tree)

0 commit comments

Comments
 (0)