code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L279

Vulnerability details

Impact

For each owner, there is a wait period requirement before they can cancel their minipool after creation.

if (block.timestamp - staking.getRewardsStartTime(msg.sender) < dao.getMinipoolCancelMoratoriumSeconds()) {
    revert CancellationTooEarly(); 
    // @audit Minipool can be cancelled immediately after creation if owner has more than 1 pool
}

However, as we can see, it used rewards start time of owner to check this wait period requirement. If owner has more than 1 pool, the second, third,... pools can be cancelled immediately after creation because duration from first pool creation to current timestamp is enough to pass minipoolCancelMoratoriumSeconds.

Proof of Concept

Consider the scenario

Alice created a minipool A at t1 = 100. Rewards start time is recorded at that momemnt rewardsStartTime = 100.
Alice created a minipool B at t2 = 200. Since 200 - rewardsStartTime = 100 already so Alice can cancelled minipool B immediately after its creation.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using another field to store creation timestamp of minipool.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #519