Skip to content

Commit 9a983b2

Browse files
LHerskindLHerskind
authored
feat: check index size in merklelib (#16585)
Please read [contributing guidelines](CONTRIBUTING.md) and remove this line. For audit-related pull requests, please use the [audit PR template](?expand=1&template=audit.md).
2 parents c5008bd + 6eb1316 commit 9a983b2

File tree

3 files changed

+206
-0
lines changed

3 files changed

+206
-0
lines changed

‎l1-contracts/src/core/libraries/Errors.sol‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ library Errors {
9797

9898
// MerkleLib
9999
error MerkleLib__InvalidRoot(bytes32 expected, bytes32 actual, bytes32 leaf, uint256 leafIndex); // 0x5f216bf1
100+
error MerkleLib__InvalidIndexForPathLength();
100101

101102
// SampleLib
102103
error SampleLib__IndexOutOfBounds(uint256 requested, uint256 bound); // 0xa12fc559

‎l1-contracts/src/core/libraries/crypto/MerkleLib.sol‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ library MerkleLib {
4747
indexAtHeight >>= 1;
4848
}
4949

50+
// Security: Ensure the index doesn't have bits set beyond the tree height
51+
// This prevents replay attacks where an attacker could use index 8 with path length 2 to walk the same path as
52+
// index 0.
53+
require(indexAtHeight == 0, Errors.MerkleLib__InvalidIndexForPathLength());
5054
require(subtreeRoot == _expectedRoot, Errors.MerkleLib__InvalidRoot(_expectedRoot, subtreeRoot, _leaf, _index));
5155
}
5256

‎l1-contracts/test/outbox/tmnt205.t.sol‎

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// Copyright 2024 Aztec Labs.
3+
pragma solidity >=0.8.27;
4+
5+
import {Test} from "forge-std/Test.sol";
6+
import {Outbox} from "@aztec/core/messagebridge/Outbox.sol";
7+
import {IOutbox} from "@aztec/core/interfaces/messagebridge/IOutbox.sol";
8+
import {Errors} from "@aztec/core/libraries/Errors.sol";
9+
import {DataStructures} from "@aztec/core/libraries/DataStructures.sol";
10+
import {Hash} from "@aztec/core/libraries/crypto/Hash.sol";
11+
import {NaiveMerkle} from "../merkle/Naive.sol";
12+
13+
contract FakeRollup {
14+
uint256 public getProvenBlockNumber = 0;
15+
16+
function setProvenBlockNum(uint256 _provenBlockNum) public {
17+
getProvenBlockNumber = _provenBlockNum;
18+
}
19+
}
20+
21+
contract Tmnt205Test is Test {
22+
using Hash for DataStructures.L2ToL1Msg;
23+
24+
address internal constant NOT_RECIPIENT = address(0x420);
25+
uint256 internal constant DEFAULT_TREE_HEIGHT = 2;
26+
uint256 internal constant AZTEC_VERSION = 1;
27+
uint256 internal constant BLOCK_NUMBER = 1;
28+
29+
FakeRollup internal rollup;
30+
Outbox internal outbox;
31+
32+
DataStructures.L2ToL1Msg[] internal $msgs;
33+
bytes32[] internal $txOutHashes;
34+
35+
bytes32 internal $root;
36+
37+
function setUp() public {
38+
rollup = new FakeRollup();
39+
outbox = new Outbox(address(rollup), AZTEC_VERSION);
40+
41+
$root = _buildWonkyTree();
42+
vm.prank(address(rollup));
43+
outbox.insert(BLOCK_NUMBER, $root);
44+
45+
rollup.setProvenBlockNum(BLOCK_NUMBER);
46+
}
47+
48+
function test_replays_exact() public {
49+
// Consume m0 at index 0.
50+
// Then try to consume it again, with index 4
51+
// If the index is not correctly validated against path length
52+
// it is possible for us to pass multiple different values
53+
// and have them all pass.
54+
55+
test_replays(1 << 2);
56+
}
57+
58+
function test_replays(uint256 _index) public {
59+
// We allow this to wander to ensure that it is not possible to get it through
60+
// with different indices either.
61+
uint256 leafIndex2 = bound(_index, 0, type(uint8).max);
62+
63+
DataStructures.L2ToL1Msg memory message = $msgs[0];
64+
uint256 leafIndex = 0;
65+
bytes32[] memory path = new bytes32[](2);
66+
path[0] = $txOutHashes[1];
67+
path[1] = $txOutHashes[2];
68+
69+
uint256 leafId = leafIndex + (1 << path.length);
70+
71+
vm.expectEmit(true, true, true, true, address(outbox));
72+
emit IOutbox.MessageConsumed(BLOCK_NUMBER, $root, message.sha256ToField(), leafId);
73+
outbox.consume(message, BLOCK_NUMBER, leafIndex, path);
74+
75+
// It should always revert, here, either incorrect values or already used.
76+
vm.expectRevert();
77+
outbox.consume(message, BLOCK_NUMBER, leafIndex2, path);
78+
}
79+
80+
function test_overrides() public {
81+
// Try to abuse the missing check to consume an invalid id, such that another
82+
// cannot consumed honestly.
83+
//
84+
// Fake the leaf index to be 8 instead of 0, so we still walk full left
85+
// But when the `leafId` is computed it will be 8 + 1 << 2 = 12
86+
// Which means that it collides with m4! Without the fix, this would allow
87+
// consuming m0 while blocking the legitimate consumption of m4.
88+
89+
// Abuser
90+
DataStructures.L2ToL1Msg memory a_message = $msgs[0];
91+
uint256 a_leafIndex = 8;
92+
bytes32[] memory a_path = new bytes32[](2);
93+
a_path[0] = $txOutHashes[1];
94+
a_path[1] = $txOutHashes[2];
95+
vm.expectRevert(Errors.MerkleLib__InvalidIndexForPathLength.selector);
96+
outbox.consume(a_message, BLOCK_NUMBER, a_leafIndex, a_path);
97+
98+
// Real message
99+
DataStructures.L2ToL1Msg memory r_message = $msgs[4];
100+
uint256 r_leafIndex = 4;
101+
bytes32[] memory r_path = new bytes32[](3);
102+
r_path[0] = $msgs[5].sha256ToField();
103+
r_path[1] = $msgs[6].sha256ToField();
104+
NaiveMerkle leftSubTree = new NaiveMerkle(1);
105+
leftSubTree.insertLeaf($txOutHashes[0]);
106+
leftSubTree.insertLeaf($txOutHashes[1]);
107+
r_path[2] = leftSubTree.computeRoot();
108+
outbox.consume(r_message, BLOCK_NUMBER, r_leafIndex, r_path);
109+
}
110+
111+
function _fakeMessage(address _recipient, uint256 _content) internal view returns (DataStructures.L2ToL1Msg memory) {
112+
return DataStructures.L2ToL1Msg({
113+
sender: DataStructures.L2Actor({
114+
actor: 0x2000000000000000000000000000000000000000000000000000000000000000,
115+
version: AZTEC_VERSION
116+
}),
117+
recipient: DataStructures.L1Actor({actor: _recipient, chainId: block.chainid}),
118+
content: bytes32(_content)
119+
});
120+
}
121+
122+
function _buildWonkyTree() internal returns (bytes32) {
123+
// Builds a wonky tree where we have a total of 7 messages over 3 txs.
124+
// outHash
125+
// / \
126+
// . tx2
127+
// / \ / \
128+
// m0 tx1 . m6
129+
// / \ / \
130+
// . m3 m4 m5
131+
// / \
132+
// m1 m2
133+
//
134+
// m0: 0 + 1 << 2 = 4
135+
// m1: 4 + 1 << 4 = 20
136+
// m2: 5 + 1 << 4 = 21
137+
// m3: 3 + 1 << 3 = 11
138+
// m4: 4 + 1 << 3 = 12
139+
// m5: 5 + 1 << 3 = 13
140+
// m6: 3 + 1 << 2 = 8
141+
142+
// Create some random messages
143+
bytes32[] memory leaves = new bytes32[](7);
144+
for (uint256 i = 0; i < 7; i++) {
145+
DataStructures.L2ToL1Msg memory message = _fakeMessage(address(this), i);
146+
leaves[i] = message.sha256ToField();
147+
148+
$msgs.push(message);
149+
}
150+
151+
// tx0 has 1 message, the message leaf is the root.
152+
$txOutHashes.push(leaves[0]);
153+
154+
// Build the subtree of tx1 with 3 message.
155+
bytes32 tx1SubtreeRoot;
156+
{
157+
NaiveMerkle subtree = new NaiveMerkle(1);
158+
subtree.insertLeaf(leaves[1]);
159+
subtree.insertLeaf(leaves[2]);
160+
tx1SubtreeRoot = subtree.computeRoot();
161+
NaiveMerkle tx1topTree = new NaiveMerkle(1);
162+
tx1topTree.insertLeaf(tx1SubtreeRoot);
163+
tx1topTree.insertLeaf(leaves[3]);
164+
$txOutHashes.push(tx1topTree.computeRoot());
165+
}
166+
167+
// Build the subtree of tx2 with 3 messages.
168+
bytes32 tx2SubtreeRoot;
169+
{
170+
NaiveMerkle tx2Subtree = new NaiveMerkle(1);
171+
NaiveMerkle tx2TopTree = new NaiveMerkle(1);
172+
tx2Subtree.insertLeaf(leaves[4]);
173+
tx2Subtree.insertLeaf(leaves[5]);
174+
tx2SubtreeRoot = tx2Subtree.computeRoot();
175+
tx2TopTree.insertLeaf(tx2SubtreeRoot);
176+
tx2TopTree.insertLeaf(leaves[6]);
177+
$txOutHashes.push(tx2TopTree.computeRoot());
178+
}
179+
180+
// First, build the left subtree with 2 txOutHashes.
181+
// subtreeRoot
182+
// / \
183+
// tx0 tx1
184+
bytes32 subtreeRoot;
185+
{
186+
NaiveMerkle subtree = new NaiveMerkle(1);
187+
subtree.insertLeaf($txOutHashes[0]);
188+
subtree.insertLeaf($txOutHashes[1]);
189+
subtreeRoot = subtree.computeRoot();
190+
}
191+
192+
// Then, build the top tree with the subtree root and the last txOutHash.
193+
// outHash
194+
// / \
195+
// subtreeRoot tx2
196+
NaiveMerkle topTree = new NaiveMerkle(1);
197+
topTree.insertLeaf(subtreeRoot);
198+
topTree.insertLeaf($txOutHashes[2]);
199+
return topTree.computeRoot();
200+
}
201+
}

0 commit comments

Comments
 (0)