ERC4626 mint uses wrong `amount`

[code423n4]

Lines of code

https://github.com/Rari-Capital/solmate/blob/8c0e278900fe552fa0739975bde21c6a07d84ccf/src/mixins/ERC4626.sol#L67

Vulnerability details

Impact

The docs/video say ERC4626.sol is in scope as its part of TurboSafe

The ERC4626.mint function mints amount 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 amount as vaults receive asset yield. Therefore, when minting, shares should be less than amount. Users receive a larger share amount here which can be exploited to drain the vault assets.

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

    // Need to transfer before minting or ERC777s could reenter.
    asset.safeTransferFrom(msg.sender, address(this), amount);
    _mint(to, amount);

    emit Deposit(msg.sender, to, amount, shares);

    afterDeposit(amount, shares);
}

POC

Assume vault.totalSupply() = 1000, totalAssets = 1500

• call mint(shares=1000). Only need to pay 1000 asset amount but receive 1000 shares => vault.totalSupply() = 2000, totalAssets = 2500.
• call redeem(shares=1000). Receive (1000 / 2000) * 2500 = 1250 amounts. Make a profit of 250 asset tokens.
• repeat until shares <> assets are 1-to-1

Recommended Mitigation Steps

In deposit:

function mint(uint256 shares, address to) public virtual returns (uint256 amount) {
-    _mint(to, amount);
+    _mint(to, shares);
}

[Joeysantoro]

https://github.com/code-423n4/2022-02-tribe-turbo-findings/issues/18

[GalloDaSballo]

The warden has identified what is most likely a small oversight, which would have drastic consequences in the internal accounting of the Vault. Because of impact, I agree with high severity.

Will make this finding primary because it shows full details and a POC.

The sponsor has mitigated