Skip to content

Commit e7d20ee

Browse files
Transpile 4d2383e1
1 parent 8aac85b commit e7d20ee

File tree

3 files changed

+35
-6
lines changed

3 files changed

+35
-6
lines changed

.changeset/shy-crews-teach.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': patch
3+
---
4+
5+
`MerkleProof`: Fix a bug in `processMultiProof` and `processMultiProofCalldata` that allows proving arbitrary leaves if the tree contains a node with value 0 at depth 1.

contracts/utils/cryptography/MerkleProofUpgradeable.sol

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,11 @@ library MerkleProofUpgradeable {
121121
// `hashes` array. At the end of the process, the last hash in the `hashes` array should contain the root of
122122
// the merkle tree.
123123
uint256 leavesLen = leaves.length;
124+
uint256 proofLen = proof.length;
124125
uint256 totalHashes = proofFlags.length;
125126

126127
// Check proof validity.
127-
require(leavesLen + proof.length - 1 == totalHashes, "MerkleProof: invalid multiproof");
128+
require(leavesLen + proofLen - 1 == totalHashes, "MerkleProof: invalid multiproof");
128129

129130
// The xxxPos values are "pointers" to the next value to consume in each array. All accesses are done using
130131
// `xxx[xxxPos++]`, which return the current value and increment the pointer, thus mimicking a queue's "pop".
@@ -146,6 +147,7 @@ library MerkleProofUpgradeable {
146147
}
147148

148149
if (totalHashes > 0) {
150+
require(proofPos == proofLen, "MerkleProof: invalid multiproof");
149151
unchecked {
150152
return hashes[totalHashes - 1];
151153
}
@@ -173,10 +175,11 @@ library MerkleProofUpgradeable {
173175
// `hashes` array. At the end of the process, the last hash in the `hashes` array should contain the root of
174176
// the merkle tree.
175177
uint256 leavesLen = leaves.length;
178+
uint256 proofLen = proof.length;
176179
uint256 totalHashes = proofFlags.length;
177180

178181
// Check proof validity.
179-
require(leavesLen + proof.length - 1 == totalHashes, "MerkleProof: invalid multiproof");
182+
require(leavesLen + proofLen - 1 == totalHashes, "MerkleProof: invalid multiproof");
180183

181184
// The xxxPos values are "pointers" to the next value to consume in each array. All accesses are done using
182185
// `xxx[xxxPos++]`, which return the current value and increment the pointer, thus mimicking a queue's "pop".
@@ -198,6 +201,7 @@ library MerkleProofUpgradeable {
198201
}
199202

200203
if (totalHashes > 0) {
204+
require(proofPos == proofLen, "MerkleProof: invalid multiproof");
201205
unchecked {
202206
return hashes[totalHashes - 1];
203207
}

test/utils/cryptography/MerkleProof.test.js

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
1-
require('@openzeppelin/test-helpers');
2-
31
const { expectRevert } = require('@openzeppelin/test-helpers');
2+
const { expect } = require('chai');
43
const { MerkleTree } = require('merkletreejs');
54
const keccak256 = require('keccak256');
65

7-
const { expect } = require('chai');
8-
96
const MerkleProof = artifacts.require('$MerkleProof');
107

118
contract('MerkleProof', function () {
@@ -176,5 +173,28 @@ contract('MerkleProof', function () {
176173
expect(await this.merkleProof.$multiProofVerify([root], [], root, [])).to.equal(true);
177174
expect(await this.merkleProof.$multiProofVerifyCalldata([root], [], root, [])).to.equal(true);
178175
});
176+
177+
it('reverts processing manipulated proofs with a zero-value node at depth 1', async function () {
178+
// Create a merkle tree that contains a zero leaf at depth 1
179+
const leaves = [keccak256('real leaf'), Buffer.alloc(32, 0)];
180+
const merkleTree = new MerkleTree(leaves, keccak256, { sortPairs: true });
181+
182+
const root = merkleTree.getRoot();
183+
184+
// Now we can pass any ** malicious ** fake leaves as valid!
185+
const maliciousLeaves = ['some', 'malicious', 'leaves'].map(keccak256).sort(Buffer.compare);
186+
const maliciousProof = [leaves[0], leaves[0]];
187+
const maliciousProofFlags = [true, true, false];
188+
189+
await expectRevert(
190+
this.merkleProof.$multiProofVerify(maliciousProof, maliciousProofFlags, root, maliciousLeaves),
191+
'MerkleProof: invalid multiproof',
192+
);
193+
194+
await expectRevert(
195+
this.merkleProof.$multiProofVerifyCalldata(maliciousProof, maliciousProofFlags, root, maliciousLeaves),
196+
'MerkleProof: invalid multiproof',
197+
);
198+
});
179199
});
180200
});

0 commit comments

Comments
 (0)