FactoryDAO contest - scaraven's results

Lines of code

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

Vulnerability details

Impact

Inside the initialize() function in MerkleVesting and MerkleResistor, if totalCoins and the length of the vesting period are not perfectly divisible, then a rounding error can occur. Usually, such a rounding error is negligible however as coinsPerSecond is multiplied by time to calculate currentWithdrawal, this effect is exaggerated. coinsPerSecond will always round down so users will not receive a fair amount of tokens as specified by the vesting schedule.

Proof of Concept

This example will use MerkleVesting though this applies to MerkleResistor as well which has a few more calculations

Say a whitelisted user calls initialize() with the correct proof and the following parameters: Let totalCoins be 50, endTime to be 30 and startTime to be 0

Calculating coinsPerSecond we get 50 / 30 = 1 (rounds down) Say the user withdraws at time t = 20 they will get 20 tokens instead of the expected 33

The user will have to wait until the end of the vesting period to get the fair share.

Recommended Mitigation Steps

Consider storing coinsPerSecond as two variables, one being the numerator and another being the denominator. The, rounding error will only occur when calculating currentWithdrawal which is less extreme and can be recovered by the end of the vesting period.