Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

Inflation attack with `BorrowingVault.sol` when the provider is Aave2 #315

Closed trungore closed 1 year ago

trungore commented 1 year ago

Title

Inflation attack with BorrowingVault.sol when the provider is Aave2

Affected smart contract

https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/providers/mainnet/AaveV2.sol#L99

Description

The ERC426 vault is vulnerable to 1 type of attack called inflation attack. You can read about it here. It can apply to our BorrowingVault.sol contract when the provider is AaveV2.sol.

Firstly let me describe some context before we come to the detail of the attack. When a user wants to deposit into x asset tokens he will get x * totalSupply() / totalAsset() shares following these lines of code. In which the totalAsset() of the vault is calculated by executing a static call getDepositBalance() to the providers. In our case, it will forward the call to contract AaveV2.sol. When the call directs to the contract Aave2.sol, it will get the correspond aTokenAddress with vault's asset and get the aToken's balance of the vault.

/// url = https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/providers/mainnet/AaveV2.sol#L88-L100
  function getDepositBalance(
    address user,
    IVault vault
  )
    external
    view
    override
    returns (uint256 balance)
  {
    IV2Pool aaveData = _getPool();
    IV2Pool.ReserveData memory rdata = aaveData.getReserveData(vault.asset());
    balance = IERC20(rdata.aTokenAddress).balanceOf(user);
  }

By observing the implementation of the function AaveV2.getDepositBalance, an attacker can totally prepare a lot of aToken and transfer it directly to the pool to manipulate the price of vault share. Read the Attack scenario for more details.

Attack scenario

The attacker will be the first user who deposits to the BorrowingVault. Here is the whole strategy.

  1. Attacker deposits 1 asset token into the vault and gets 1 share. Right now the price of a share is 1
  2. Attacker prepares a lot of aToken corresponding to the asset token (he can get this by interacting directly with AaveV2).
  3. An innocent user named Bob wants to deposit x asset tokens into the vault. He should be received x shares with the current price. Unluckily,
  4. The attacker front-run Bob's deposit transaction by transferring directly to the vault x aToken without using the function deposit / mint. After this action, vault.totalAsset() = 1 + x.
  5. At the time the transaction of Bob is executed, he will get:
    shares = x * totalSupply() / totalAsset()
           = x * 1 / (1 + x)
           = 0

    ==> Bob loses his x asset tokens and receives 0 shares in return.

  6. Attacker then can call withdraw to get his 2x + 1 asset tokens and make x asset tokens profit.

PR: https://github.com/Fujicracy/fuji-v2/pull/334

Recommendation

Following the technique used in UniswapV2, which mints for address(0) some CONSTANT liquidity to make it unable to manipulate the price of the vault https://github.com/Uniswap/v2-core/blob/ee547b17853e71ed4e0101ccfd52e70d5acded58/contracts/UniswapV2Pair.sol#L121

Note that this issue also applies to the compoundV2 provider. Since the fix above will cover the attack of compound too, so I just consider those 2 provider identical.

0xdcota commented 1 year ago

This issue will be closed in PR #498