privacy-scaling-explorations/zk-kit

Missing checks to ensure 'zero' is less than SNARK_SCALAR_FIELD

kcharbo3 opened this issue · 0 comments

Description
During initialization of the IncrementalBinaryTree.sol, the user can enter any value for the uint256 zero field. This becomes an issue if the user uses a value greater than the SNARK_SCALAR_FIELD. A zero leaf can be inside an array of proofSiblings when proving existence of a leaf which may cause an issue when the IncrementalBinaryTree.verify function is called during the IncrementalBinaryTree.remove function. The verify function requires that all proofSiblings are less than the SNARK_SCALAR_FIELD:
require(proofSiblings[i] < SNARK_SCALAR_FIELD, "IncrementalBinaryTree: sibling node must be < SNARK_SCALAR_FIELD" );
https://github.com/appliedzkp/zk-kit/blob/main/packages/incremental-merkle-tree.sol/contracts/IncrementalBinaryTree.sol#L127

So, if the 'zero' leaf is greater than the SNARK_SCALAR_FIELD, this verify function will unintentionally fail.

Possible Fix
Add a require(zero < SNARK_SCALAR_FIELD, "...") statement in the IncrementalBinaryTree.init() function.
https://github.com/appliedzkp/zk-kit/blob/main/packages/incremental-merkle-tree.sol/contracts/IncrementalBinaryTree.sol#L29