Badger Citadel contest - 0x1f8b's results

Lines of code

https://github.com/code-423n4/2022-02-badger-citadel/blob/84596551d62f243d13fcb2d486346dde08002f7b/contracts/TokenSaleUpgradeable.sol#L96-L131 https://github.com/code-423n4/2022-02-badger-citadel/blob/84596551d62f243d13fcb2d486346dde08002f7b/contracts/TokenSaleUpgradeable.sol#L355-L369 https://github.com/code-423n4/2022-02-badger-citadel/blob/84596551d62f243d13fcb2d486346dde08002f7b/contracts/TokenSaleUpgradeable.sol#L144-L205

Vulnerability details

Impact

The contract is vulnerable to rogue pool attack.

Proof of Concept

The owner is not forced to transfer the out tokens in any moment, the contract must have the balance for call finalize, but if the investors buy with the hope of claim, and the owner never send the out tokens, the owner will be able to take the inTokens without any loss, just calling sweep.

Recommended Mitigation Steps

• Won't be able to call sweep without finalize
• Ensure funds of tokenOut during initialize

Non-critical

Open TODO

The code that contains "open todos" reflects that the development is not finished and that the code can change a posteriori, prior release, with or without audit

Source reference:

• contracts\TokenSaleUpgradeable.sol:13

Low

Local openzeppelin sources

The openzeppelin imports used reflect that they use local paths, so it is possible that these code have been modified, are obsolete or they can contain vulnerabilities, being out of scope they could not be audited, but without a doubt it is a bad practice not to use them of official packages

Source reference:

• contracts\TokenSaleUpgradeable.sol: 4-7

Rule changing during the game

There are a lot of methods that allow the owner to modify the rules of the game while the token sale is open, and while the investors have already made the decision to invest or not according to the current rules.

The following functions should only be called BEFORE the output has been started

Source reference:

• contracts\TokenSaleUpgradeable.sol: [setTokenOutPrice,setGuestlist,setTokenInLimit,setTokenOutPrice,setSaleDuration]

Contract management risk

A possible risk involved with the admin account is that they are in a position to buy all the sale, because during the initialize method the _saleStart can be set exactly to the current block date (line 107), and it is also possible to call buy on the current block date (line 149). So we can conclude that it is possible to initialize and buy all the token sale in the same block.

Source reference:

• contracts\TokenSaleUpgradeable.sol: 107, 149