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

1 stars 1 forks source link

PermissionlessBasicPoolFactory use hard coded decimals of 18 #283

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/PermissionlessBasicPoolFactory.sol#L169-L169

Vulnerability details

Once reward/deposit tokens decimals differ from 18 the calculations with a hard coded 1e18 will become grossly incorrect.

This will lead either to receiving no rewards: say deposit is USDC with decimals of 6, being divided by 1e18 it adds 1e-12 to the rewards calculations, making it close to zero.

When it is the opposite it's more severe as first reward takers will simply empty the pool.

Proof of Concept

Rewards calculation mixes up the reward and deposit token amounts as if both have decimals of 18:

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

   rewardsLocal[i] = (secondsDiff * pool.rewardsWeiPerSecondPerToken[i] * receipt.amountDepositedWei) / 1e18;

getMaximumRewards also uses hard coded 1e18:

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

return pool.rewardsWeiPerSecondPerToken[rewardIndex] * pool.maximumDepositWei * (pool.endTime - pool.startTime) / 1e18;

Recommended Mitigation Steps

Record and take into account reward and deposit token decimals.

illuzen commented 2 years ago

Duplicate #47

gititGoro commented 2 years ago

Reducing severity as rewards represent leakage, not deposits