`ERC4626.mint()` doesn't mint the correct amount

[code423n4]

Lines of code

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

Vulnerability details

Impact

The ERC4626.mint() function doesn't mint the correct amount of tokens. Instead of minting amount number of tokens, it should mint shares number of tokens.

Since the user doesn't receive the correct amount of tokens I'd rate this issue "HIGH".

Proof of Concept

    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);
    }

The amount value represents the number of tokens the user has to transfer to the ERC4626 contract while the shares value represents the number of tokens they should receive in return. The current implementation mints amount number of tokens.

In the opposite function, withdraw() you can see the correct logic:

    function redeem(
        uint256 shares,
        address to,
        address from
    ) public virtual returns (uint256 amount) {
        uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals.

        if (msg.sender != from && allowed != type(uint256).max) allowance[from][msg.sender] = allowed - shares;

        // Check for rounding error since we round down in previewRedeem.
        require((amount = previewRedeem(shares)) != 0, "ZERO_ASSETS");

        beforeWithdraw(amount, shares);

        _burn(from, shares);

        emit Withdraw(from, to, amount, shares);

        asset.safeTransfer(to, amount);
    }

The user specifies the amount of tokens they want to redeem and are quoted the number of tokens they will get back.

Or here in the deposit function:

    function deposit(uint256 amount, address to) public virtual returns (uint256 shares) {
        // Check for rounding error since we round down in previewDeposit.
        require((shares = previewDeposit(amount)) != 0, "ZERO_SHARES");

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

        _mint(to, shares);

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

        afterDeposit(amount, shares);
    }

User specifies the number of tokens they want to deposit through amount and the function computes the number of tokens that should be minted, shares.

Tools Used

none

Recommended Mitigation Steps

Replace amount with shares:

    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, shares);

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

        afterDeposit(amount, shares);
    }

[Joeysantoro]

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

[GalloDaSballo]

Duplicate of #20