code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

proposeTokenWhitelisting can be DoS #685

Closed c4-bot-2 closed 8 months ago

c4-bot-2 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L167

Vulnerability details

Impact

The attacker made proposeTokenWhitelisting impossible to create.

Proof of Concept

    function proposeTokenWhitelisting( IERC20 token, string calldata tokenIconURL, string calldata description ) external nonReentrant returns (uint256 _ballotID)
    {
        require( address(token) != address(0), "token cannot be address(0)" );
        require( token.totalSupply() < type(uint112).max, "Token supply cannot exceed uint112.max" ); // 5 quadrillion max supply with 18 decimals of precision

@>      require( _openBallotsForTokenWhitelisting.length() < daoConfig.maxPendingTokensForWhitelisting(), "The maximum number of token whitelisting proposals are already pending" );
        require( poolsConfig.numberOfWhitelistedPools() < poolsConfig.maximumWhitelistedPools(), "Maximum number of whitelisted pools already reached" );
        require( ! poolsConfig.tokenHasBeenWhitelisted(token, exchangeConfig.wbtc(), exchangeConfig.weth()), "The token has already been whitelisted" );

        string memory ballotName = string.concat("whitelist:", Strings.toHexString(address(token)) );

        uint256 ballotID = _possiblyCreateProposal( ballotName, BallotType.WHITELIST_TOKEN, address(token), 0, tokenIconURL, description );
@>      _openBallotsForTokenWhitelisting.add( ballotID );
        return ballotID;
    }

ProposeTokenWhitelisting will check _openBallotsForTokenWhitelisting.length() whether more than daoConfig.maxPendingTokensForWhitelisting. But the question is anyone can call this function, the attacker can create invalid propose, let _openBallotsForTokenWhitelisting. Length more than the maximum value. proposeTokenWhitelisting could no longer be executed.

Because the daoConfig.maxPendingTokensForWhitelisting defaults to 5, so _openBallotsForTokenWhitelisting. The length is more than the maximum easily.

    uint256 public maxPendingTokensForWhitelisting = 5;

Tools Used

vscode, manual

Recommended Mitigation Steps

    function proposeTokenWhitelisting( IERC20 token, string calldata tokenIconURL, string calldata description ) external nonReentrant returns (uint256 _ballotID)
    {
        require( address(token) != address(0), "token cannot be address(0)" );
        require( token.totalSupply() < type(uint112).max, "Token supply cannot exceed uint112.max" ); // 5 quadrillion max supply with 18 decimals of precision

-       require( _openBallotsForTokenWhitelisting.length() < daoConfig.maxPendingTokensForWhitelisting(), "The maximum number of token whitelisting proposals are already pending" );
        require( poolsConfig.numberOfWhitelistedPools() < poolsConfig.maximumWhitelistedPools(), "Maximum number of whitelisted pools already reached" );
        require( ! poolsConfig.tokenHasBeenWhitelisted(token, exchangeConfig.wbtc(), exchangeConfig.weth()), "The token has already been whitelisted" );

        string memory ballotName = string.concat("whitelist:", Strings.toHexString(address(token)) );

        uint256 ballotID = _possiblyCreateProposal( ballotName, BallotType.WHITELIST_TOKEN, address(token), 0, tokenIconURL, description );
        _openBallotsForTokenWhitelisting.add( ballotID );
        return ballotID;
    }

Assessed type

DoS

c4-judge commented 9 months ago

Picodes marked the issue as primary issue

c4-sponsor commented 9 months ago

othernet-global (sponsor) confirmed

othernet-global commented 8 months ago

There is now no limit to the number of tokens that can be proposed for whitelisting.

Also, any whitelisting proposal that has reached quorum with sufficient approval votes can be executed.

https://github.com/othernet-global/salty-io/commit/ccf4368fcf1777894417fccd2771456f3eeaa81c

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory

c4-judge commented 8 months ago

Picodes marked issue #116 as primary and marked this issue as a duplicate of 116

c4-judge commented 8 months ago

Picodes marked the issue as duplicate of #991