howlbot-integration[bot] commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/cb72b1e9053c02a58d874ff376359a83dc3f0742/hardhat-vultisig/contracts/Whitelist.sol#L204-L228

Vulnerability details

Impact

User can bypass whitelist functionality because it doesn't work as intended

Proof of Concept

Whenever a buy of VULT tokens is initiated during the whitelist period, the _beforeTokenTransfer is called:

function _beforeTokenTransfer(address from, address to, uint256 amount) internal override {

        if (_whitelistContract != address(0)) {
            IWhitelist(_whitelistContract).checkWhitelist(from, to, amount);
        }

        super._beforeTokenTransfer(from, to, amount);

    }

which calls the function checkWhitelist in Whitelist.sol:

    /// @notice Check if address to is eligible for whitelist
    /// @param from sender address
    /// @param to recipient address
    /// @param amount Number of tokens to be transferred
    /// @dev Check WL should be applied only
    /// @dev Revert if locked, not whitelisted, blacklisted or already contributed more than capped amount
    /// @dev Update contributed amount

    function checkWhitelist(address from, address to, uint256 amount) external onlyVultisig {

        if (from == _pool && to != owner()) {

            // We only add limitations for buy actions via uniswap v3 pool
            // Still need to ignore WL check if it's owner related actions
            if (_locked) {
                revert Locked();
            }

            if (_isBlacklisted[to]) {
                revert Blacklisted();
            }

            if (_allowedWhitelistIndex == 0 || _whitelistIndex[to] > _allowedWhitelistIndex) {
                revert NotWhitelisted();
            }

            // // Calculate rough ETH amount for VULT amount
            uint256 estimatedETHAmount = IOracle(_oracle).peek(amount);

            if (_contributed[to] + estimatedETHAmount > _maxAddressCap) {
                revert MaxAddressCapOverflow();
            }

            _contributed[to] += estimatedETHAmount;
        }

    }

As said by the comments above, the function should revert if a user is not whitelisted, however this is exactly where the bug generates from.

Focus on this line here:

            if (_allowedWhitelistIndex == 0 || _whitelistIndex[to] > _allowedWhitelistIndex) {
                revert NotWhitelisted();
            }

Firstly, _allowedWhitelistIndex is set by the owner to indicate which whitelist indexes are able to pass this check for example:

_allowedWhitelistIndex = 5 So users with whitelist indexes <= 5 can buy vult tokens.

But a user that hasn't been whitelisted - his index will be 0, so this right here:

_whitelistIndex[to] > _allowedWhitelistIndex

Doesn't hold, a non whitelisted user can still buy vult tokens from the pool as long as _allowedWhitelistIndex != 0

Tools Used

Manual Review

Recommended Mitigation Steps

Make a check to ensure that _whitelistIndex[to] != 0

Assessed type

Context

c4-judge commented 4 months ago

alex-ppg changed the severity to 3 (High Risk)