Wrong implementation of BaseVault.previewMint

Title

Affected smart contract

https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/abstracts/BaseVault.sol#L265-L267

Description

Function BaseVault.mint() is implemented as follows:

/// url = https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/abstracts/BaseVault.sol#L351-L364
  function mint(uint256 shares, address receiver) public virtual override returns (uint256) {
    uint256 assets = previewMint(shares);

    if (shares + totalSupply() > maxMint(receiver)) {
      revert BaseVault__mint_moreThanMax();
    }
    if (assets < minAmount) {
      revert BaseVault__mint_lessThanMin();
    }

    _deposit(_msgSender(), receiver, assets, shares);

    return assets;
  }

The function firsly calculates the amount of assets the user needs to provide for the contract to mint a given shares. Unfornately the previewMint call use a round-down calculation which might let the attacker deposit less than the corresponding amount needed.

/// url = https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/abstracts/BaseVault.sol#L265-L267
  function previewMint(uint256 shares) public view virtual override returns (uint256) {
    return _convertToAssets(shares, Math.Rounding.Down);
  }

/// url = https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/abstracts/BaseVault.sol#L513-L524
  function _convertToAssets(
    uint256 shares,
    Math.Rounding rounding
  )
    internal
    view
    virtual
    returns (uint256 assets)
  {
    uint256 supply = totalSupply();
    return (supply == 0) ? shares : shares.mulDiv(totalAssets(), supply, rounding);
  }

You can take at a look at ERC4626.sol of OZ, they use the round-up when calculating the corresponding mint

Attack scenario

We shall assume the state of vault as follows:

totalSupply() = 200
totalAssets() = 100

The whole strategy as follows:

Attacker call mint(3, attacker_address, attacker_address) He will get 3 shares and need to provide 3 * 100 / 200 = 1
Attacker keep calling mint(3, attacker_address, attacker_address) He will get 3 shares and need to provide 3 * (100 + 1) / 203 = 1
Attacker keep calling mint(3, attacker_address, attacker_address) He will get 3 shares and need to provide 3 * (100 + 2) / 206 = 1
Attacker call redeem(9, attacker_address, attacker_address) He will get 9 * (100 + 3) / (200 + 9) = 4 asset token in return

In step 1 and step 2 and 3, attacker will pay totally 3 asset tokens, but when we look at step 4, attacker can get back 4 asset tokens. Attacker can keep doing this process many times to get as much tokens as he can.

PR for the attack test: https://github.com/Fujicracy/fuji-v2/pull/300

Recommendation

Modify function previewMint() as follows:

function previewMint(uint256 shares) public view virtual override returns (uint256) {
    return _convertToAssets(shares, Math.Rounding.Down);
  }

Fujicracy / fuji-v2