search

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

12 stars 7 forks source link

Permit does not revert for tokens that do not implement it. #468

Closed code423n4 closed 11 months ago

code423n4 commented 12 months ago

Lines of code

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

Vulnerability details

Impact

Callers should not rely on permit to revert for arbitrary tokens especially if permit is used as a security check. Tokens which do not revert on permit either do not implement it or have a non-reverting fallback function. Most notable among them is WETH. In the case of Pool Together, permit is used to allow the vault to make deposit's on behalf of users but this call will always fail because the approval part of the permit is never executed, thereby bricking a main functionality of the protocol.

Proof of Concept

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1103 More details on this kind of vulnerability can be found here

Tools Used

Manual Review

Recommended Mitigation Steps

Assessed type

ERC20

c4-judge commented 11 months ago

Picodes marked the issue as unsatisfactory: Overinflated severity

Picodes commented 11 months ago

This is quite clear in my opinion from the implementation