TokenInLimit can be set higher than what the contract has to sell

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/KnightingRound.sol#L380

Vulnerability details

Impact

If totalTokenOutBought > tokenOut.balanceOf(address(this)) all tokens will be unclaimable and all funds would be lost.

Proof of Concept

TokenInLimit can be set arbitrarily high even if the contract doesn't have enough tokens to sell. https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/KnightingRound.sol#L380

Then users can buy more TokenOut than the contract balance. If the balance is smaller than TokenOutBought the finalize function will revert. https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/KnightingRound.sol#L275

Calling the finalize function is a prerequisite to claiming bought tokens. Then everyone would lose their bought tokens.

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/KnightingRound.sol#L210

This issue was already sent in a previous badgerCitadel contest but wasn't fixed. Since the sponsor didn't state this as a known issue I consider this to still be valid.

Recommended Mitigation Steps

Setting tokenInLimit must have restrictions linked to the contract tokenOut balance to protect from overselling.

[GalloDaSballo]

I think the saleRecipient receives the tokens before the end of the sale, but to claim you have to wait until the end and then call claim.

As such the balance of the contract (which could be indicative of liquidity) is not that important as it can be changed at the time of claiming

[jack-the-pug]

The sale can only be finalized when tokenOut.balanceOf(address(this)) >= totalTokenOutBought

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/KnightingRound.sol#L275-L278