sherlock-audit / 2022-10-astaria-judging

6 stars 1 forks source link

Bnke0x0 - ERC4626 mint uses the wrong amount #1

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 2 years ago

Bnke0x0

high

ERC4626 mint uses the wrong amount

Summary

ERC4626 mint uses the wrong amount

Vulnerability Detail

Impact

The ERC4626-Cloned.mint function mints assets instead of shares. This will lead to issues when the asset <> shares are not 1-to-1 as will be the case for most vaults over time. Usually, the asset amount is larger than the share assets amount as vaults receive the asset Astaria. Therefore, when minting, shares should be less than the amount of the assets. Users receive a larger share amount here which can be exploited to drain the vault assets.

https://github.com/sherlock-audit/2022-10-astaria/blob/main/lib/astaria-gpl/src/ERC4626-Cloned.sol#L334

  '_mint(receiver, shares);'

Code Snippet

https://github.com/sherlock-audit/2022-10-astaria/blob/main/lib/astaria-gpl/src/ERC4626-Cloned.sol#L324-L339

    '  function mint(uint256 shares, address receiver)
     public
     virtual
     returns (uint256 assets)
    {
    assets = previewMint(shares); // No need to check for rounding error, previewMint rounds up.

   // Need to transfer before minting or ERC777s could reenter.
   ERC20(underlying()).safeTransferFrom(msg.sender, address(this), assets);

  _mint(receiver, shares);

   emit Deposit(msg.sender, receiver, assets, shares);

   afterDeposit(assets, shares);
   }'

Assume vault.totalSupply() = 1000totalAssets = 1500

Tool used

Manual Review

Recommendation

In deposit:

      'function mint(uint256 shares, address receiver) public virtual returns (uint256 assets) {
       -    _mint(receiver, assets);
       +    _mint(receiver, shares);
       }'
androolloyd commented 1 year ago

I believe the mint method is always dealing and shares and deposits convert assets into shares for minting. I will look to confirm this as the original underlying code comes from the solmate library directly.