sherlock-audit / 2023-02-fair-funding-judging

1 stars 0 forks source link

hickuphh3 - Vault deposits are not disabled if migration has been activated / executed #90

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

hickuphh3

medium

Vault deposits are not disabled if migration has been activated / executed

Summary

Deposits remain enabled if migration has been activated, and even after execution.

Vulnerability Detail

Since vault potentially holds an Alchemix position over a long time during which changes at Alchemix could happen, the migration_admin has complete control over the vault and its position after giving depositors a 30 day window to liquidate (or transfer with a flashloan) their position if they're not comfortable with the migration.

During this 30 day window, and even after migration has been executed, the deposit flow may still work (depending if Alchemix disables deposits on their end).

Impact

Deposits could theoretically still be possible during the 30 day window / after migration has been executed.

Code Snippet

https://github.com/sherlock-audit/2023-02-fair-funding/blob/main/fair-funding/contracts/Vault.vy#L198-L232

Tool used

Manual Review

Recommendation

To err on the side of caution, consider reverting if migration has been activated, or executed.

# in register_deposit()
assert self.migration_active == max_value(uint256), "migration activated"
Unstoppable-DeFi commented 1 year ago

Conceptually the auction phase of a Fair Funding campaign will be limited and rather short (for example first Fair Funding campaign will run 16 days / 16 auctions). Compared to the available APYs on Alchemix (2-5% max), the discrepancy between early depositors and late depositors is effectively near zero and negligible.

hrishibhat commented 1 year ago

Closing the issue based on Sponsor comment.