Badger Citadel contest - harleythedog's results

Lines of code

https://github.com/code-423n4/2022-02-badger-citadel/blob/84596551d62f243d13fcb2d486346dde08002f7b/contracts/TokenSaleUpgradeable.sol#L254

Vulnerability details

Impact

The admins are the only ones that are able to finalize the sale. Throughout the token buying process, the admins are transferred tokenIn tokens, and the users are expecting to be able to get the corresponding tokenOut tokens once the sale is finalized. The admin can grief the users by never calling finalize, and then the users will not be able to claim their tokens. This isn't necessarily a gain for the admins, but the ability for the admins to take tokenIn tokens and the users never receive their corresponding tokenOut tokens is an issue. Even though the admins may be well intentioned, it is best to mitigate ways that the admins can negatively affect the users, and this is a major way the admins can grief the users.

Proof of Concept

See code for finalize here. Notice that it is only callable by an admin.

Tools Used

Inspection.

Recommended Mitigation Steps

There should be a few extra logic checks in the finalize function (e.g. making sure that a predetermined amount of tokens have been purchased). Then, the finalize function should have the onlyOwner modifier removed, as then there would be no way for the admins to grief users out of all of the tokenOut tokens that they are owed.