code-423n4 / 2022-05-factorydao-findings

1 stars 1 forks source link

Multiple vestings for the same user will fail #249

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleVesting.sol#L115

Vulnerability details

Impact

Loss of funds from multiple vestings for a single user

Proof of Concept

In MerkleVesting and MerkleResistor vestings are distributed using merkle trees. Creators of the vesting submit the Merkle root of the tree and deposit the funds to be distributed. A Merkle leaf in merkleVesting is defined by the following parameters : (destination, totalCoins, startTime, endTime, lockPeriodEndTime).

A tree could have many different leaves with the same destination but different vesting parameters. If that is done then users won't be able to claim their vesting since after claiming one initialized[destination][treeIndex] will be true.

If the tree creator sent the funds correctly then the user won't be able to claim his vestings and funds will be stuck inside the contract. https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleVesting.sol#L106

Recommended Mitigation Steps

Check the same leaf can't be used twice but not only with the destination. The mapping could be made to work with destination and start times or amounts. In that way the same user will be able to claim many vestings with different amounts / start times.

illuzen commented 2 years ago

Duplicate #148

gititGoro commented 2 years ago

Reducing severity