search

code-423n4 / 2023-08-pooltogether-mitigation-findings

0 stars 0 forks source link

M-23 MitigationConfirmed #8

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

Vulnerability details

Comments

The ERC4626 spec states that maxDeposit returns the max amount of assets that can be deposited, and similarly maxMint must return the maximum number of shares that can be minted. Since the V5 vaults connect to underlying ERC4626 yield vaults, the V5 vault is constrained by the maxDeposit and maxMint of the external yield vault; this should be reflected in the respective methods of the V5 vault.

Mitigation

The updated implementation now does 2 additional checks. Firstly it verifies that the amount of assets/shares (converted to shares) doesn’t exceed type(uint96).max since this is the type used to store balances in the TwabController. Secondly it also checks maxMint and maxDeposit of the underlying yield vault and then caps the mint/deposit at the minimum of the two possible values. This change ensures the contract is now properly ERC-4626 compliant and resolves the original issue. This mitigation overlaps significantly with M-04.

Conclusion

LGTM

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory