Salty.IO - forgebyola's results

Lines of code

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

Vulnerability details

Impact

The Dao allows users to create proposals for token whitelisting in proposals.proposeTokenWhitelisting. This function only requires the user to have a certain percentage of staked xSalt to propose a token for whitelisting. The daoConfig provides a maximum number of tokenWhitelist proposals that can be created at any time in daoConfig.maximumTokensForWhitelisting which ranges from 3 - 12. In the proposals.proposeTokenWhitelisting function there is a check which ensures the proposals._openBallotsForTokenWhitelisting is always less than the maximumTokensForWhitelisting or the function reverts. If all checks pass, the proposal is added to the proposals._openBallotsForTokenWhitelisting.

File: src/dao/Proposals.sol

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

Due to this check, a malicious user may create enough junk proposals to fill up the proposals._openBallotsForTokenWhitelisting using multiple accounts with sufficient staked salt. Afterward, if a legitimate user attempts to create a proposal for token whitelisting with this function, it reverts. This causes both sufficient potential grief to users and the protocol as well as a Denial of Service as this prevents proper functioning of the intended protocol logic of complete decentralization. Although, this process may be expensive, it would not stop a determined bad actor or competitor.

Proof of Concept

• Malicious user Bob wants to prevent actual ballots for token whitelisting from being created. He sets up a bot contract which creates multiple addresses and stakes salt on each of the addresses to meet the daoConfig.requiredProposalPercentStakeTimes1000(). This can be ensured by reading directly from the daoConfig contract as well as the stakingRewards.totalShares() contract to calculate this value.
• Bob then creates a proposal with each of these addresses, with a new ballotName everytime required to pass the check when creating proposals.
• By reading from daoConfig.maximumTokensForWhitelisting, Bob knows exactly how many proposals is necessary at all times. Also, proposals._openBallotsForTokenWhitelisting lets Bob know if this ever falls below the maximumTokensForWhiteListing - 1. This bot can be set up to create more junk proposals as soon as anyone passes or is rejected. This can be prolonged for as long as Bob desires or based on available resources, effectively this can be maintained for even years. Therefore, during this period no new proposals can be created by legitimate users.

Tools Used

Manual Review

Recommended Mitigation Steps

1. Only add the proposal to the proposals._openBallotsForTokenWhitelisting after confirming the proposal by the Dao in proposals.createConfirmationProposal.

File: src/dao/Proposals.sol

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

File: src/Proposals.sol
function createConfirmationProposal( string calldata ballotName, BallotType ballotType, address address1, string calldata string1, string calldata description ) external returns (uint256 ballotID)
		{
		require( msg.sender == address(exchangeConfig.dao()), "Only the DAO can create a confirmation proposal" );

+		ballotId = _possiblyCreateProposal( ballotName, ballotType, address1, 0, string1, description );
+		_openBallotsForTokenWhitelisting.add( ballotID );
- 		return  _possiblyCreateProposal( ballotName, ballotType, address1, 0, string1, description );
		}

This would ensure that any number of token whitelisting proposal can be created but only when they are confirmed by the dao would they be added to the openBallot. Now if Bob creates multiple junk proposals it does not affect the protocol or other users as these proposals would need to be confirmed first by the dao before they are added to the openBallot.

Assessed type

DoS