Lines of code

Vulnerability details

The ERC4626RouterBase contract contains a set of functions that act as wrappers for a ERC4626 contract, providing a base periphery functionality around a ERC4626 vault.

There are a number of different flaws in the wrapped implementations of mint, deposit, withdraw and redeem.

Impact

All four functions present in the contract are marked as payable (because of the IERC4626RouterBase interface definitions) but don’t need to handle ETH payments, as these operate on the assumption that the underlying asset is an ERC20 token. This will cause loss of funds if a user sends ETH to any of the functions.

Note that these functions are exposed as payable functions in the AstariaRouter contract, as this contract inherits from ERC4626Router.

##mint

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

The mint function uses shares as the amount to ERC20.approve but this value is wrong, as shares is the output of the vault. It should first call previewMint to get the amount of the underlying token needed to mint that number of shares. Compare that amount against the maxAmountIn and then call ERC20.approve with that value.

##withdraw and redeem

function withdraw(
  IERC4626 vault,
  address to,
  uint256 amount,
  uint256 maxSharesOut
) public payable virtual override returns (uint256 sharesOut) {

  ERC20(address(vault)).safeApprove(address(vault), amount);
  if ((sharesOut = vault.withdraw(amount, to, msg.sender)) > maxSharesOut) {
    revert MaxSharesError();
  }
}

<https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626RouterBase.sol#L55-L66&gt;

function redeem(
  IERC4626 vault,
  address to,
  uint256 shares,
  uint256 minAmountOut
) public payable virtual override returns (uint256 amountOut) {

  ERC20(address(vault)).safeApprove(address(vault), shares);
  if ((amountOut = vault.redeem(shares, to, msg.sender)) < minAmountOut) {
    revert MinAmountError();
  }
}

Both function calls to ERC20.approve are not valid, since ERC4626.withdraw will burn the shares and no transfers of shares are needed.

Recommendations

• In all cases, since functions are marked as payable, verify the value sent is 0 require(msg.value == 0) to prevent accidental loss of funds.

• For mint, use previewMint to know how much assets are needed to mint the given shares:

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

• For withdraw and redeem, remove the call to ERC20.approve

  function withdraw(
  IERC4626 vault,
  address to,
  uint256 amount,
  uint256 maxSharesOut
  ) public payable virtual override returns (uint256 sharesOut) {
  require(msg.value == 0);
  if ((sharesOut = vault.withdraw(amount, to, msg.sender)) > maxSharesOut) {
  revert MaxSharesError();
  }
  }

  function redeem(
  IERC4626 vault,
  address to,
  uint256 shares,
  uint256 minAmountOut
  ) public payable virtual override returns (uint256 amountOut) {
  require(msg.value == 0);
  if ((amountOut = vault.redeem(shares, to, msg.sender)) < minAmountOut) {
  revert MinAmountError();
  }
  }

---

The text was updated successfully, but these errors were encountered:

All reactions