code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

[WP-H5] `yVault.sol` A malicious early user/attacker can manipulate the vault's pricePerShare to take an unfair share of future users' deposits #156

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/yVault.sol#L142-L157

Vulnerability details

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/yVault.sol#L142-L157

function deposit(uint256 _amount) public noContract(msg.sender) {
    require(_amount > 0, "INVALID_AMOUNT");
    uint256 balanceBefore = balance();
    token.safeTransferFrom(msg.sender, address(this), _amount);
    uint256 supply = totalSupply();
    uint256 shares;
    if (supply == 0) {
        shares = _amount;
    } else {
        //balanceBefore can't be 0 if totalSupply is > 0
        shares = (_amount * supply) / balanceBefore;
    }
    _mint(msg.sender, shares);

    emit Deposit(msg.sender, _amount);
}

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/yVault/yVault.sol#L166-L184

function withdraw(uint256 _shares) public noContract(msg.sender) {
    require(_shares > 0, "INVALID_AMOUNT");

    uint256 supply = totalSupply();
    require(supply > 0, "NO_TOKENS_DEPOSITED");

    uint256 backingTokens = (balance() * _shares) / supply;
    _burn(msg.sender, _shares);

    // Check balance
    uint256 vaultBalance = token.balanceOf(address(this));
    if (vaultBalance < backingTokens) {
        uint256 toWithdraw = backingTokens - vaultBalance;
        controller.withdraw(address(token), toWithdraw);
    }

    token.safeTransfer(msg.sender, backingTokens);
    emit Withdrawal(msg.sender, backingTokens);
}

A malicious early user can deposit() with 1 wei of asset token as the first depositor of the Vault, and get 1 wei of shares token.

Then the attacker can send 10000e18 - 1 of asset tokens and inflate the price per share from 1.0000 to an extreme value of 1.0000e22 ( from (1 + 10000e18 - 1) / 1) .

As a result, the future user who deposits 19999e18 will only receive 1 wei (from 19999e18 * 1 / 10000e18) of shares token.

They will immediately lose 9999e18 or half of their deposits if they withdraw() right after the deposit().

Recommendation

Consider requiring a minimal amount of share tokens to be minted for the first minter, and send a port of the initial mints as a reserve to the DAO so that the pricePerShare can be more resistant to manipulation.

spaghettieth commented 2 years ago

Duplicate of #12