github/zk-kit
1
1
Fork 0
mirror of https://github.com/privacy-scaling-explorations/zk-kit.git synced 2026-04-22 03:00:15 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix: add checks for new leaves in the update function

Browse Source 
This bug was found by Veridise during their audit of Semaphore.

fix #35
This commit is contained in:
cedoor
2022-12-13 14:10:46 +01:00
parent d3b87736d9
commit 27284d16bc
4 changed files with 44 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
packages/incremental-merkle-tree.sol/contracts/IncrementalBinaryTree.sol
View File

@@ -92,6 +92,8 @@ library IncrementalBinaryTree {
uint256[] calldata proofSiblings,
uint8[] calldata proofPathIndices
) public {
require(newLeaf != leaf, "IncrementalBinaryTree: new leaf cannot be the same as the old one");
require(newLeaf < SNARK_SCALAR_FIELD, "IncrementalBinaryTree: new leaf must be < SNARK_SCALAR_FIELD");
require(
verify(self, leaf, proofSiblings, proofPathIndices),
"IncrementalBinaryTree: leaf is not part of the tree"

2
packages/incremental-merkle-tree.sol/contracts/IncrementalQuinTree.sol
View File

@@ -108,6 +108,8 @@ library IncrementalQuinTree {
uint256[4][] calldata proofSiblings,
uint8[] calldata proofPathIndices
) public {
require(newLeaf != leaf, "IncrementalQuinTree: new leaf cannot be the same as the old one");
require(newLeaf < SNARK_SCALAR_FIELD, "IncrementalQuinTree: new leaf must be < SNARK_SCALAR_FIELD");
require(
verify(self, leaf, proofSiblings, proofPathIndices),
"IncrementalQuinTree: leaf is not part of the tree"

24
packages/incremental-merkle-tree.sol/test/IncrementalBinaryTreeTest.ts
View File

@@ -91,11 +91,29 @@ describe("IncrementalBinaryTreeTest", () => {
await expect(transaction).to.be.revertedWith("IncrementalBinaryTreeTest: tree does not exist")
})
it("Should not update a leaf if its value is > SNARK_SCALAR_FIELD", async () => {
const leaf = BigInt("21888242871839275222246405745257275088548364400416034343698204186575808495618")
it("Should not update a leaf if the new value is the same as the old one", async () => {
const leaf = BigInt(3)
const transaction = contract.updateLeaf(treeId, leaf, leaf, [0, 1], [0, 1])
await expect(transaction).to.be.revertedWith(
"IncrementalBinaryTree: new leaf cannot be the same as the old one"
)
})
it("Should not update a leaf if its new value is > SNARK_SCALAR_FIELD", async () => {
const leaf = BigInt("21888242871839275222246405745257275088548364400416034343698204186575808495618")
const transaction = contract.updateLeaf(treeId, BigInt(3), leaf, [0, 1], [0, 1])
await expect(transaction).to.be.revertedWith("IncrementalBinaryTree: new leaf must be < SNARK_SCALAR_FIELD")
})
it("Should not update a leaf if its original value is > SNARK_SCALAR_FIELD", async () => {
const leaf = BigInt("21888242871839275222246405745257275088548364400416034343698204186575808495618")
const transaction = contract.updateLeaf(treeId, leaf, BigInt(4), [0, 1], [0, 1])
await expect(transaction).to.be.revertedWith("IncrementalBinaryTree: leaf must be < SNARK_SCALAR_FIELD")
})
@@ -114,7 +132,7 @@ describe("IncrementalBinaryTreeTest", () => {
const { pathIndices, siblings } = tree.createProof(2)
const transaction = contract.updateLeaf(
treeId,
leaf,
BigInt(4),
leaf,
siblings.map((s) => s[0]),
pathIndices

22
packages/incremental-merkle-tree.sol/test/IncrementalQuinTreeTest.ts
View File

@@ -94,11 +94,27 @@ describe("IncrementalQuinTreeTest", () => {
await expect(transaction).to.be.revertedWith("IncrementalQuinTreeTest: tree does not exist")
})
it("Should not update a leaf if its value is > SNARK_SCALAR_FIELD", async () => {
const leaf = BigInt("21888242871839275222246405745257275088548364400416034343698204186575808495618")
it("Should not update a leaf if the new value is the same as the old one", async () => {
const leaf = BigInt(3)
const transaction = contract.updateLeaf(treeId, leaf, leaf, [[0, 1, 2, 3]], [0])
await expect(transaction).to.be.revertedWith("IncrementalQuinTree: new leaf cannot be the same as the old one")
})
it("Should not update a leaf if its new value is > SNARK_SCALAR_FIELD", async () => {
const leaf = BigInt("21888242871839275222246405745257275088548364400416034343698204186575808495618")
const transaction = contract.updateLeaf(treeId, BigInt(3), leaf, [[0, 1, 2, 3]], [0])
await expect(transaction).to.be.revertedWith("IncrementalQuinTree: new leaf must be < SNARK_SCALAR_FIELD")
})
it("Should not update a leaf if its original value is > SNARK_SCALAR_FIELD", async () => {
const leaf = BigInt("21888242871839275222246405745257275088548364400416034343698204186575808495618")
const transaction = contract.updateLeaf(treeId, leaf, BigInt(4), [[0, 1, 2, 3]], [0])
await expect(transaction).to.be.revertedWith("IncrementalQuinTree: leaf must be < SNARK_SCALAR_FIELD")
})
@@ -115,7 +131,7 @@ describe("IncrementalQuinTreeTest", () => {
tree.update(2, leaf)
const { pathIndices, siblings } = tree.createProof(2)
const transaction = contract.updateLeaf(treeId, leaf, leaf, siblings, pathIndices)
const transaction = contract.updateLeaf(treeId, BigInt(4), leaf, siblings, pathIndices)
await expect(transaction).to.be.revertedWith("IncrementalQuinTree: leaf is not part of the tree")
})