FIRST DEPOSITOR ATTACK IS PRESENT IN THE `Vault.sol` CONTRACT

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L407-L415

Vulnerability details

Impact

The Vault.deposit() function is vulnerable to first depositor attack. It can be described as follows:

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

Then the first depositor can send 10000e18 - 1 of asset tokens directly to the contract 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 (from 19999e18 * 1 / 10000e18) 1 share token.

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

Proof of Concept

  function deposit(uint256 _assets, address _receiver) public virtual override returns (uint256) {
    if (_assets > maxDeposit(_receiver))
      revert DepositMoreThanMax(_receiver, _assets, maxDeposit(_receiver));

    uint256 _shares = _convertToShares(_assets, Math.Rounding.Down);
    _deposit(msg.sender, _receiver, _assets, _shares);

    return _shares;
  }

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L407-L415

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Consider requiring a minimal amount of share tokens to be minted for the first minter, and send a part of the initial mints as a reserve to a DAO like reserve or burn, so that the price per share can be more resistant to manipulation.

Assessed type

Other

[Picodes]

Sending assets directly to the contract would basically be giving them to the next depositor here.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid