code-423n4 / 2024-06-vultisig-findings

2 stars 0 forks source link

Whitelist can be bypassed #128

Closed howlbot-integration[bot] closed 5 months ago

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#L216-L219

Vulnerability details

In the function Whitelist::checkWhitelist, it is intended to assert whether an address is whitelisted for receiving tokens from the pool. If an address is not whitelisted, is blacklisted, or has already contributed more than the capped amount, this method should revert. However, it is missing a check to verify if the address is whitelisted, which should be indicated by a value greater than zero.

Impact

The impact of this omission is that any user can receive tokens from the pool, regardless of whether they are whitelisted or not. This could lead to various issues, including unauthorized users receiving tokens and potential exploitation of the pool.

Proof of concept

If we go through the function below, as you can see is missing the check is _whitelistIndex[to] > 0

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
            ) {
                // audit-high missing zero check
                revert NotWhitelisted();
            }

            // // Calculate rough ETH amount for VULT amount
            uint256 estimatedETHAmount = IOracle(_oracle).peek(amount); // if peek result zero, this will
            if (_contributed[to] + estimatedETHAmount > _maxAddressCap) {
                revert MaxAddressCapOverflow();
            }

            _contributed[to] += estimatedETHAmount;
        }
    }

Tools Used

Manual review

Recommended Mitigation Steps

Add a check to verify that the _whitelistIndex[to] value is greater than zero to ensure that the address is whitelisted before proceeding with the rest of the function logic. The updated function should look like this:

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();
        }

        // Check if the address is whitelisted
+       if (_whitelistIndex[to] == 0) {
+           revert NotWhitelisted();
+       }

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

        // Calculate rough ETH amount for VULT amount
        uint256 estimatedETHAmount = IOracle(_oracle).peek(amount);
        if (estimatedETHAmount == 0) {
            revert InvalidAmount();
        }

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

        _contributed[to] += estimatedETHAmount;
    }
}

Assessed type

ERC20

c4-judge commented 4 months ago

alex-ppg marked the issue as satisfactory