Fee-on-transfer underlying deposit asset token conflict with ERC4626

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/main/src/AstariaRouter.sol#L123-L153
https://github.com/code-423n4/2023-01-astaria/blob/main/src/PublicVault.sol#L233-L265
https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626-Cloned.sol#L19-L52

Vulnerability details

Impact

When depositing capital to selected PublicVault.sol via deposit() or mint(), the underlying ERC20 token amount transferred to the contract proportionately determines the minted amount of ERC-4626 VaultTokens that represent the liquidity provider's share in the vault. However, if the ERC20 token entailed is deflationary, it could lead to accounting errors resulting in the last batch of liquidity providers unable to withdraw their funds due to insufficient contract balance.

Proof of Concept

File: ERC4626-Cloned.sol#L19-L52

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

    require(shares > minDepositAmount(), "VALUE_TOO_SMALL");
    // Need to transfer before minting or ERC777s could reenter.
    ERC20(asset()).safeTransferFrom(msg.sender, address(this), assets);

    _mint(receiver, shares);

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

    afterDeposit(assets, shares);
  }

  function mint(
    uint256 shares,
    address receiver
  ) public virtual returns (uint256 assets) {
    assets = previewMint(shares); // No need to check for rounding error, previewMint rounds up.
    require(assets > minDepositAmount(), "VALUE_TOO_SMALL");
    // Need to transfer before minting or ERC777s could reenter.
    ERC20(asset()).safeTransferFrom(msg.sender, address(this), assets);

    _mint(receiver, shares);

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

    afterDeposit(assets, shares);
  }

super.deposit() and super.mint() are unanimously used by deposit() and mint() in AstariaRouter.sol and PublicVault.sol to correspondingly invoke the above parental functions.

As can be seen in the code block, no measure has been made to either stem or cater for fee-on-transfer tokens.

Tools Used

Manual inspection

Recommended Mitigation Steps

Consider:

1. whitelisting the underlying deposit asset ensuring no fee-on-transfer token is allowed when deploying a new PublicVault from AstariaRouter.sol, or
2. calculating the balance before and after the transfer, and use the difference between those two balances as the amount rather than using the input amount if deflationary token is going to be allowed in the protocol.

[c4-judge]

Picodes marked the issue as duplicate of #51

[c4-judge]

Picodes marked the issue as satisfactory