Submitted by AuditsAreUS
https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleLib.sol#L36-L42
https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleDropFactory.sol#L94
The size of a leaf is the same size of the parent data that is hashed, both are 64 bytes. As a result it is possible to have a hash collision between a leaf and any node in the tree. This allows for proofs to be repeated multiple times by taking subtrees as leaves.
Fraudulent proofs will disrupt the airdrop and transfer funds to invalid addresses.
For example consider the following binary tree which has 4 leaves d,e,f,g
a->b
a->c
b->d
b->e
c->f
c->g
To calculate the parent hash for c it is keccak(f || g) and the parent hash for a is keccak(b || c) as seen in the parentHash() function below.
A leaf is calculated as
abi.encode(address,uint) will output 64 bytes. Since abi.encode(bytes32,bytes32) will also be 64 bytes it is possible to have a hash collision between a leaf and a parent node.
Taking the example above if we now set destination = keccak(e || d) and value = keccak(f || g) and provide the proof as an empty array since we are already at a, the root []. This proof will verify for destination and value set to the hash of each child node.
This issue is rated as medium as there are some drawbacks to the attack that will make it challenging to pull off in practice. The first is that destination is a 20 bytes address and thus will require the node in the tree to have 12 leading zero bytes which may not occur. Second is the value is transferred to the user and so it is likely that the balance of the contract will not be sufficient for this transfer to succeed.
Consider using leaf = keccak(abi.encodePacked(destination, value)) in withdraw() as this will reduce the size of the leaf data being hashed to 52 bytes.
Since keccak256 prevents length extension attacks a different length of data to be hashed can be assumed to give different hashes and prevent a collision between a leaf and other nodes in the tree.
illuzen (FactoryDAO) acknowledged and commented:
Justin Goro (judge) commented:
