Summary

Due to the check if (amount > maxRecoverAmount) { revert CommonEventsAndErrors.InvalidParam(); }, the protocol will run into a state where the last token recovery logic as intended fails and such tokens are lost because of the flawed recovery logic.

Vulnerability Details

The function SpiceAuction::recoverTokens() is used to recover auction tokens for the last but not started auction. Due to the check if (amount > maxRecoverAmount) { revert CommonEventsAndErrors.InvalidParam(); } the amount available to recover always resolves to 0. There are a couple ways this state is reached but let's start with one of them which is a malicious user holding out on claiming tokens (please note that this state would still be reached regardless but we are using this scenario to illustrate the issue better).

Scenario

1. Protocol starts an auction to distribute 200 tokens in the SpiceAuction contract

2. Alice uses multiple addresses or just one address to get an allocation of all 200 tokens

3. The current epoch ends and claims go live

4. Alice claims 100 tokens and leaves the rest 100 tokens unclaimed

5. Protocol starts another auction to distribute e.g 500 tokens

6. Protocol decides to recover the 500 tokens as auction has not yet started

7. At this point, the calculation: uint256 totalAuctionTokenAllocation = _totalAuctionTokenAllocation[token]; will be 700 tokens, uint256 balance = IERC20(token).balanceOf(address(this)); will be 600 tokens since Alice only claimed 100 and protocol sent 500 more in, uint256 maxRecoverAmount = balance - (totalAuctionTokenAllocation - _claimedAuctionTokens[token]); will be 0 because the calculation resolves as such: 600 - (700 - 100) = 600 - 600 = 0. Hence, any amount passed in as the recoverToken argument will fail because any positive amount will be greater than 0 at that point if (amount > maxRecoverAmount) { revert CommonEventsAndErrors.InvalidParam(); }

8. Alice keeps waiting to claim until the current epoch is now elapsed and the protocol cannot call the recoverToken function again.

9. At this point, protocol might decide to call startAuction without first sending auction tokens to utilize the 500 tokens inside the SpiceAuction contract but that would also not work because of the calculation below:

Hence 500 auction tokens are lost.

Impact

The recoverToken is being used to recover tokens for the very last but not yet started epoch, I believe this is a high-severity issue since the tokens can never be recovered at this point and are lost.

Tools Used

Manual Review

Recommendations

To avoid this situation, we recommend a couple of ways of which some are:

1. Changing the logic of claiming tokens by users. When a user claims, all of his tokens are sent making partial claims impossible.

2. When recovering tokens, don't account for unclaimed user balances

3. When recovering tokens, recover only the amount which belongs to the last epoch we are trying to recover since at this point no user claims would have happened for it because the auction never even got started in the first place.