Astaria contest - Bjorn_bug's results

Lines of code

https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626-Cloned.sol#L19

Vulnerability details

Impact

As arbitrary ERC20 tokens can be deposited in the liquidity pool to earn yield and mint VaultTokens to acquire shares, and since there is no indication in the documentation that the protocol will not support fee-on-transfer tokens or rebasing/deflationary/inflationary tokens in the future. Therefore, the "assets" parameter should be calculated each time to make sure that the amount of "assets" received is excluding any fees. This will prevent any potential abuse in minting more shares than intended.

Proof of Concept

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

Tools Used

Inspection

Recommended Mitigation Steps

Consider Using the balance before and after the transfer to calculate the received amount of assets instead of assuming that it would be equal to the amount passed as a parameter.