Wrong amount of allownace approval

[code423n4]

Lines of code

https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626RouterBase.sol#L21

Vulnerability details

Impact

Detailed description of the impact of this finding.
Wrong amount of allowance approval for the mint() function, which leads to two consequences: the mint() function might fail each time due to insufficient allowance approval; or allowance approved is unnecessary large subject to future exploitation.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626RouterBase.sol#L21
The following allowance approval amount is shares, this is wrong since it needs to approve the amount of assets corresponds to those shares.

function mint(
    IERC4626 vault,
    address to,
    uint256 shares,
    uint256 maxAmountIn
  ) public payable virtual override returns (uint256 amountIn) {
    ERC20(vault.asset()).safeApprove(address(vault), shares);
    if ((amountIn = vault.mint(shares, to)) > maxAmountIn) {
      revert MaxAmountError();
    }
  }

Tools Used

Remix

Recommended Mitigation Steps

We need to approve the amount of assets that corresponds to those shares, see below:

function mint(
    IERC4626 vault,
    address to,
    uint256 shares,
    uint256 maxAmountIn
  ) public payable virtual override returns (uint256 amountIn) {
    uint256 assets = previewMint(shares);
    ERC20(vault.asset()).safeApprove(address(vault), assets);      // @audit: we need approve ``assets`` rather than ``shares``
    if ((amountIn = vault.mint(shares, to)) > maxAmountIn) {
      revert MaxAmountError();
    }
  }

[c4-judge]

Picodes marked the issue as primary issue

[SantiagoGregory]

@androolloyd

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked issue #488 as primary and marked this issue as a duplicate of 488

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

This previously downgraded issue has been upgraded by Picodes

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as duplicate of #488