Astaria contest - eierina's results

Lines of code

https://github.com/AstariaXYZ/astaria-gpl/blob/f430f5b8fe868d4883a8249555df76ee7752587a/src/ERC4626-Cloned.sol#L129-L133

Vulnerability details

Impact

The first transfer in a vault that is based on ERC4626Cloned produces different results depending on whether it is done via deposit or via mint. While first deposit produces a number of shares that is 1:1 with the asset, in the case of mint, the results may vary:

• if the user mints 1 share, he will transfer 10 ethers of the asset setting a ratio of share:asset of 1 : 10 ethers
• if the user mints 10 ethers of shares, he will transfer 10 ethers of the asset setting a ration of share:asset of 1:1
• if the user mints 20 ethers of share, he will transfer 10 ethers of the asset setting a ratio of share:asset of 2:1

This means that if the first mint is for a small number of shares (say N), the minimum deposit from there on will be given by 10 ethers / N or in other words the minimum transfer to produce at least 1 share will be very large.

Proof of Concept

    function testLargeTransferZeroShares() public {
        TestNFT nft = new TestNFT(1);
        address tokenContract = address(nft);
        uint256 tokenId = uint256(0);

        uint256 initialBalance = WETH9.balanceOf(address(this));

        // create a PublicVault with a 14-day epoch
        address publicVault =
            _createPublicVault({strategist: strategistOne, delegate: strategistTwo, epochLength: 14 days});

        PublicVault pv = PublicVault(publicVault);
        address joe = address(0x70a57ed);
        vm.deal(joe, 20 ether);
        vm.startPrank(joe);
        WETH9.deposit{value: 20 ether}();
        WETH9.approve(publicVault, 20 ether);
        uint256 assets = pv.mint(1, joe);

        vm.expectRevert(bytes("ZERO_SHARES"));
        pv.deposit(9.99 ether, joe);

        vm.stopPrank();
    }

Test results:

Running 1 test for src/test/AstariaTest.t.sol:AstariaTest
[PASS] testLargeTransferZeroShares() (gas: 1351952)
Test result: ok. 1 passed; 0 failed; finished in 13.69ms

Tools Used

n/a

Recommended Mitigation Steps

Mind and deposit previews are calculated using the following formulas.

previewMint(uint256 shares)    => totalSupply() == 0 ? 10e18 : shares * totalAssets() / totalSupply();
previewDeposit(uint256 assets) => totalSupply() == 0 ? assets : assets * totalSupply() / totalAssets();

Either substitute `10e18` with the original `shares` in previewMint or ensure that if totalSupply is zero, then the number of shares requested (or issued) is also a large, matching value in order to produce a 1:1 initial ratio.

Lines of code

https://github.com/AstariaXYZ/astaria-gpl/blob/f430f5b8fe868d4883a8249555df76ee7752587a/src/ERC4626-Cloned.sol#L38-L52

Vulnerability details

Impact

If a user or a contract that has a large allowance (10 ethers or max) on an ERC4626Cloned based Vault that has not yet received any deposits, calls mint with 0 share argument, will have a 10 ethers of the asset transferred to the Vault with no way to reclaim it because he/it will receive no shares.

NOTE: Raising this issue as high risk conforming to judging criteria '3 — High: Assets can be stolen/*lost*/compromised directly'. No complains if judge feels that the necessary pre-conditions makes it a '2 — Med'.

Proof of Concept

    function testAssetsLostToVault() public {
        TestNFT nft = new TestNFT(1);
        address tokenContract = address(nft);
        uint256 tokenId = uint256(0);

        uint256 initialBalance = WETH9.balanceOf(address(this));

        // create a PublicVault with a 14-day epoch
        address publicVault =
            _createPublicVault({strategist: strategistOne, delegate: strategistTwo, epochLength: 14 days});

        PublicVault pv = PublicVault(publicVault);
        address joe = address(0x70a57ed);
        vm.deal(joe, 10 ether);
        vm.startPrank(joe);
        WETH9.deposit{value: 10 ether}();
        WETH9.approve(publicVault, 10 ether);
        uint256 assets = pv.mint(0, joe);

        emit log_string(string.concat("Joe's balance     : ", Strings.toString(WETH9.balanceOf(address(joe)))));
        emit log_string(string.concat("Joe's shares      : ", Strings.toString(pv.balanceOf(address(joe)))));
        emit log_string(string.concat("pv.totalAssets()  : ", Strings.toString(pv.totalAssets())));
        emit log_string(string.concat("pv.totalSupply()  : ", Strings.toString(pv.totalSupply())));
        emit log_string(string.concat("pv.previewRedeem(): ", Strings.toString(pv.previewRedeem(pv.balanceOf(address(joe))))));

        vm.stopPrank();
    }

Test results:

Running 1 test for src/test/AstariaTest.t.sol:AstariaTest
[PASS] testAssetsLostToVault() (gas: 1307633)
Logs:
  Joe's balance     : 0
  Joe's shares      : 0
  pv.totalAssets()  : 10000000000000000000
  pv.totalSupply()  : 0
  pv.previewRedeem(): 0

Test result: ok. 1 passed; 0 failed; finished in 29.48ms

Tools Used

n/a

Recommended Mitigation Steps

Since the previewMint does not use the original supply == 0 ? shares : ..., at least add a require precondition to the mint function to prevent minting of 0 shares.