Badger Citadel contest - Jujic's results

Handle

Jujic

Vulnerability details

1. Long Revert Strings

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met. ##Proof of Concept

require(
            _saleStart >= block.timestamp,
            "TokenSale: start date may not be in the past"
        );

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L108

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L112

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L114

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L117

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L274

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L290

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L304

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L318

Tools

https://planetcalc.com/9029/

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

2. Adding unchecked directive can save gas

Impact

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

Proof of Concept

https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L232

function getTokenInLimitLeft() external view returns (uint256 limitLeft_) {
        if (totalTokenIn < tokenInLimit) {
            limitLeft_ = tokenInLimit - totalTokenIn;
        }
    }

Tools

Remix

Recommended Mitigation Steps

Consider using 'unchecked' where it is safe to do so. Example:

unchecked {
         limitLeft_ = tokenInLimit - totalTokenIn;
 }

3. Caching variables

Impact

Some of the variable can be cached to slightly reduce gas usage.

Proof of Concept

saleStart and guestlist can be cached. https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L144-L186

Tools

Remix

Recommended Mitigation Steps

Consider caching those variable for read and make sure write back to storage.

4. Use Minimal Interface for gas optimizations

Vulnerability details

Impact

ERC20Upgradeable is a superset of ERC20 and in this case, only ERC20 functions are required to be called on, so IERC20 can be used. ##Proof of Concept https://github.com/code-423n4/2022-02-badger-citadel/blob/08d43b5662f6d6c3707b720eadc773b913f79f2c/contracts/TokenSaleUpgradeable.sol#L19-L21

 ERC20Upgradeable public tokenOut;
 ERC20Upgradeable public tokenIn;

Tools

Remix

Recommended Mitigation Steps