Badger Citadel contest - floppydisk's results

Impact

saleDuration is not validated to ensure that saleStart + saleDuration < (2**65-1). An invalid saleDuration will cause a revert anytime the buy(), saleEnded(), and finalize() functions are called, rendering the contract non-functional.

In the contract's current state, this does not pose a danger to user or owner funds. Since the buy() function is not executable, tokenIn assets cannot be transferred into the contract. Similarly, since tokenOut assets can always be swept() by the owner, all assets can be transferred out and the contract can be unwound.

However, if there were an architectural change to the contract such that swept() could not be called prior to finalized being set to true, any tokenOut assets transferred in would be frozen.

Proof of Concept

Update the DURATION variable in TokenSaleUpgradeable.t.sol to 2**64-1.

Currently, the following test will fail a result of this overflow error: testExtendSaleDuration

[FAIL. Reason: Arithmetic over/underflow] testExtendSaleDuration() (gas: 5508)

Tools Used

Forge fuzz tests on user-supplied inputs

Recommended Mitigation Steps

Issue can be resolved with the implementation of a validation check during the initialize() function so invalid saleDuration values are reverted.

The following validation at #118 will suffice: require(_saleStart < _saleStart + _saleDuration, "invalid sale period");