Lines of code
<https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L52&gt;

Vulnerability details

Impact

• In case fCash has not matured yet, convertToShares() may return incorrect value due to division round down 2 times. It may leads to the case that user need more amount of share than expected to withdraw assets.

• In wfCashERC4626.convertToShares() function, it calculated amout of share from assets amount. And it used a formula which round down the division (1st one).

  return (assets * totalSupply()) / totalAssets();

• But in totalAsset() function, it also rounded down 1 more time (2nd one) when fCash has not matured

  int256 pvExternal = (pvInternal * precision) / Constants.INTERNAL_TOKEN_PRECISION;

Proof Of Concept

1. Assume underlyingToken is USDC with decimals = 6 so precision = 1e6. Wrapped fCash has totalSupply() = 2000000, has not matured and pvInternal = 2111111. We try to get amount of share with assets = 1e9.

2. Use 2nd formula

pvExternal = (pvInternal * precision) / INTERNAL_TOKEN_PRECISION
= (2111111 * 1e6) / 1e8
= 21111
=> totalAssets() = 21111

3. Use first formula, we get

(assets * totalSupply()) / totalAssets()
= (1e9 * 2000000) / 21111
= 94737340722

4. But if we didn’t round down in first calculation, we will get

(assets * totalSupply()) / totalAssets() 
= (assets * totalSupply()) / ((pvInternal * precision) / INTERNAL_TOKEN_PRECISION)
= (assets * totalSupply() * INTERNAL_TOKEN_PRECISION) / (pvInternal * precision)
= (1e9 * 2000000 * 1e8) / (2111111 * 1e6)
= 94736847091

Tools Used

Manual Review

Recommended Mitigation Steps

Merge 2 formula into 1 calculation to improve precision.

The text was updated successfully, but these errors were encountered:

All reactions