Attacker can steal vault funds through the deposit function.

[code423n4]

Lines of code

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

Vulnerability details

Impact

In the deposit function, a check is made to see if the amount of assets being deposited by the user is greater than the amount of assets the vault currently holds. The vault then transfers the difference between the assets being deposited and the vault’s assets if the condition is true, otherwise it simply transfers the assets to the Yield Vault. However, the assets transferred in this situation come from the vault and not the attacker. An attacker could use this to get the vault to deposit assets and mint shares to them without contributing a single asset to the vault. The attacker could then withdraw their minted shares from the vault and profit from the exploit.

Proof of Concept

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

Tools Used

Manual review and Foundry for tests

Recommended Mitigation Steps

Getting rid of the conditional statement is advised. The deposit function should always make a transfer of funds from the user before a deposit to the Yield Vault is made.

Assessed type

Token-Transfer

[Picodes]

As the Vault is not supposed to hold funds, this is made in order to allow routers or other contracts to transfer the funds in advance.

[c4-judge]

Picodes marked the issue as unsatisfactory: Insufficient proof