Salty.IO - josephdara's results

Lines of code

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

Vulnerability details

Impact

Only whitelisted tokens can be used within the DAO, however the whitelisting process can be DOSed by malicious users using multiple addresses. Few facts about whitelisting tokens:

• Any user can submit a whitelisttoken proposal
• Only the top voted token, ABOVE the quorum gets whitelisted
• There is an array for pending whitelist token proposal: _openBallotsForTokenWhitelisting;
• There is a cap on the total number of tokens that can be proposed to be whitelisted, max possible is 12
• If memecoins/shitcoins/malicious tokens are proposed to fill this 12, they will never pass the quorum
• Only top voted tokens that have passed the quorum gets removed from the _openBallotsForTokenWhitelisting

Proof of Concept

In the function below, the length of the Enumerable array is checked:

	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
//@audit can dos whitelist process
		require( _openBallotsForTokenWhitelisting.length() < daoConfig.maxPendingTokensForWhitelisting(), "The maximum number of token whitelisting proposals are already pending" );
			

In the finalizeBallot() function in the DAO.sol It first checks for the quorum using

		require( proposals.canFinalizeBallot(ballotID), "The ballot is not yet able to be finalized" );

Then it verifies the approval in _finalizeTokenWhitelisting() using:

		if ( proposals.ballotIsApproved(ballotID ) )

Then it also verifies if it is the top in _finalizeTokenWhitelisting() using:

			// Fail to whitelist for now if this isn't the whitelisting proposal with the most votes - can try again later.
			uint256 bestWhitelistingBallotID = proposals.tokenWhitelistingBallotWithTheMostVotes();
			require( bestWhitelistingBallotID == ballotID, "Only the token whitelisting ballot with the most votes can be finalized" );

Even if the protocol is willing to whitelist a malicious token, which would require people to vote it to the top. The malicious user can simply propose another contract with another address.

Tools Used

Manual Review

Recommended Mitigation Steps

Create Proposal method to remove address from the _openBallotsForTokenWhitelisting, or create an admin function to do this.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/ManagedWallet.sol#L59-L69

Vulnerability details

Impact

The receive() function in the ManagedWallet contract is used to approve address changes. By sending greater than or equal to 0.05 ether to the contract, however after the confirmation, the confirmationWallet can DOS the proposed Wallet forever by continuously sending 0.05 ether every 29 days. This prevents finalization as it continues shifting the activeTimelock by 30 days on every call

Proof of Concept

In the receive function, the contract increases the timelock every time the confirmation wallet sends it over 0.05 ether.

	// The confirmation wallet confirms or rejects wallet proposals by sending a specific amount of ETH to this contract
    receive() external payable
    	{
    	require( msg.sender == confirmationWallet, "Invalid sender" );

		// Confirm if .05 or more ether is sent and otherwise reject.
		// Done this way in case custodial wallets are used as the confirmationWallet - which sometimes won't allow for smart contract calls.
		//@audit Medium  confirmation wallet can DOS wallet change
    	if ( msg.value >= .05 ether )
    		activeTimelock = block.timestamp + TIMELOCK_DURATION; // establish the timelock

Hence the activeTimelock never ends.

Tools Used

Manual Review

Recommended Mitigation Steps

Create a boolean value which returns true everytime the confirmation wallet has confirmed. it should return false after change or after rejection and it should be require in receive()

Assessed type

DoS