search

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

1 stars 1 forks source link

`PermissionlessBasicPoolFactory` does not support non-18 decimals token. Yield give 0 reward #207

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Hardcode 1e18 for all rewards token can cause addPool issue with rewards/reposit token like WBTC (8 decimals) or USDC (6 decimals). The underlying function getMaximumRewards will return 0 or smaller rewards than expected due to precision lost.

Impact

Normally this would not be a problem because maximumDepositWei or rewardsWeiPerSecondPerToken will be token with 18 decimals large enough to handle hardcode 1e18 division.

But if user want user to deposit WBTC and rewards both USDC and promoted token (either with 18 decimals or not), then this issue will occur. (Yield reward per second for USDC is really high but received rewards is 0)

Unexpected contract behaviour + Hypothetical scam = Medium severity.

Proof Of Concept

  1. Add pool with MyToken(18 decimals) and USDC(8 decimals) as token rewards for depositing WBTC.
  2. Max deposit WBTC should be 10 WBTC (1e9) over the period of 1 month (2592000 ~= 2e6). Rewards 100K USDC (1e12) rewardsWeiPerSecondPerToken = 1e12 / 2e6 ~= 5e5.
  3. During fundPool call. MyToken have no problem, but getMaximumRewards for USDC will be calculated as 5e5 * 1e9 * 2e6 / 1e18 = 1e21 / 1e18 = 1000 = 0.001 USDC.
  4. Only 0.001 USDC is transfered as reward for the pool. Plus, lots of MyToken that might be worth something.

The scam use case for this pretty much inflated price for meme/DAO token. Since no USDC is rewarded, the scammer only have to mint their token to give out. Little expense, free exposure. No user lost their WBTC.

Migitation

The easiest way is replacing hardcode / 1e18 with depositToken.decimals(). Note: still not handle meme token case like there is only 1 token with only 1e18 in circulation.

This is up to user to handle.

illuzen commented 2 years ago

Duplicate #47

itsmetechjay commented 2 years ago

Re-closing as duplicate.