`ChangeAdapter` can lead to a DOS if the new adapter breaches the `maxDeposit` limit when assets are migrated from old adapter

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/Vault.sol#L612

Vulnerability details

Impact

ChangeAdapter migrates all assets from existing adapter and deposits them into new adapter - however, at the stage of proposing & changing adapters, it is not checked whether deposit into new adapter can breach the max deposit limit. For eg. Yearn adapter has a maxDeposit depending on the limit of yearn vaults - if such limit is breached, adapter can never be changed until excess deposits are shifted to another vault - this logic seems to be missing currently.

Proof of Concept

• Yearn vault has a limit of 1 million USDC
• Owner wants to shift from Beefy to Yearn Vault a balance of 2 million
• Proposed change is accepted
• But actual change gets rejected because current deposit > max allowed deposit

Tools Used

Manual

Recommended Mitigation Steps

• Either check if a vault can accept deposits at proposal stage (while this is not fool proof, as assets can still be deposited during quit period, but it atleast acts as a good first level check)
• Or have a logic to send excess assets to a secondary vault

[c4-judge]

dmvt marked the issue as primary issue

[c4-sponsor]

RedVeil marked the issue as sponsor acknowledged

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[c4-judge]

dmvt marked the issue as grade-b