search

code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Vault funds can be stolen by a malicious Yield Vault. #462

Closed code423n4 closed 12 months ago

code423n4 commented 12 months ago

Lines of code

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

Vulnerability details

Impact

When a vault is initialized, it sets Max Token Approval for the Yield Vault which allows the Yield Vault to ALWAYS have access to the funds in the vault. Since vaults can be created by anyone as long as they provide an ERC-4626 compliant yield source, an attacker could set up a malicious ERC-4626 contract and set that as the yield source for a newly created Vault. The attacker could then have the malicious contract use SafeTransferFrom to periodically empty the vault of assets that haven't yet been sent to the malicious yield vault.

Proof of Concept

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

Tools Used

Manual review

Recommended Mitigation Steps

Vaults should only approve tokens when they are being transferred out.

Assessed type

Token-Transfer

c4-judge commented 12 months ago

Picodes marked the issue as duplicate of #324

c4-judge commented 11 months ago

Picodes changed the severity to 2 (Med Risk)

c4-judge commented 11 months ago

Picodes marked the issue as satisfactory