Popcorn contest - ayeslick's results

Lines of code

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

Vulnerability details

Impact

A creator can propose an adapter or fees with the standard quitPeriod then after a day has passed the creator can call change setQuitPeriod to change quitPeriod to 1 day. The creator can then call changeAdapter or changeFees updating the adapter or fees before the expected date.

Proof of Concept

quitPeriod == 5 days

Creator proposes a new adapter or fees

After a day creator calls setQuitPeriod setting quitPeriod to 1 day from 5 days

Admin calls changeAdapter or changeFees immediately changing the adapter or fees 4 days early.

Recommended Mitigation Steps

Prevent the creator from changing rageQuit until the currently proposed change is complete

Lines of code

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

Vulnerability details

Impact

proposedFees and proposedFeeTime are initially set to their default values. They can be set by the creator of the vault through the vault controller. When a vault is deployed it can be deployed with a fee already set. Once these values are set and the quitPeriod has passed anyone can call the changeFees function. If someone calls changeFee before the creator calls proposeFees via the vault controller for the first time, fees will be set to proposedFees which is 0. proposedFeeTime is 0 so the check on line 541, if (block.timestamp < proposedFeeTime + quitPeriod), is bypassed.

Proof of Concept

creator deploys vault with fees set to 5% an operator calls changeFee check on line 541 is bypassed because block.timestamp is greater than quitPeriod fees is set to 0

Recommended Mitigation Steps

If proposedFeeTime == 0 revert();