Salty.IO - Tripathi's results

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L317 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L385 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/DAO.sol#L278

Vulnerability details

Impact

Altering the quorum for a proposal could result in unexpected outcomes since, for all active proposals, the current quorum is considered instead of the quorum that was present at the start of the proposal.

Proof of Concept

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/DAO.sol#L278

The finalizeBallot() function is called to execute the proposal, checking if the minimum time and quorum for the proposal are met using proposals.canFinalizeBallot(ballotID).

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L385

canFinalizeBallot() examines the ballotMinimumNedTime from the ballot storage variable. For quorum, it assumes that the quorum remains the same as it was at the time of proposing the proposal. However, the current quorum depends on various parameters.

	function requiredQuorumForBallotType( BallotType ballotType ) public view returns (uint256 requiredQuorum)
		{
		// The quorum will be specified as a percentage of the total amount of SALT staked
		uint256 totalStaked = staking.totalShares( PoolUtils.STAKED_SALT );
		require( totalStaked != 0, "SALT staked cannot be zero to determine quorum" );

		if ( ballotType == BallotType.PARAMETER )
			requiredQuorum = ( 1 * totalStaked * daoConfig.baseBallotQuorumPercentTimes1000()) / ( 100 * 1000 );
		else if ( ( ballotType == BallotType.WHITELIST_TOKEN ) || ( ballotType == BallotType.UNWHITELIST_TOKEN ) )
			requiredQuorum = ( 2 * totalStaked * daoConfig.baseBallotQuorumPercentTimes1000()) / ( 100 * 1000 );
		else
			// All other ballot types require 3x multiple of the baseQuorum
			requiredQuorum = ( 3 * totalStaked * daoConfig.baseBallotQuorumPercentTimes1000()) / ( 100 * 1000 );

		// Make sure that the requiredQuorum is at least 0.50% of the total SALT supply.
		// Circulating supply after the first 45 days of emissions will be about 3 million - so this would require about 16% of the circulating
		// SALT to be staked and voting to pass a proposal (including whitelisting) 45 days after deployment..
		uint256 totalSupply = ERC20(address(exchangeConfig.salt())).totalSupply();//@audit increase total supply will increase the quorum
		uint256 minimumQuorum = totalSupply * 5 / 1000;

		if ( requiredQuorum < minimumQuorum )
			requiredQuorum = minimumQuorum;
		}

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L317

The issue arises as the quorum is global for all proposals, leading to the execution of proposals with incorrect quorum values

1. Decreasing quorum may lead to executing proposals, which was intentionally ignored due to less participation
2. opposite to above

Proposer could wait to decrease/increase to call finalize ballot depending upon his/her pro and cons

Tools Used

Manual

Recommended Mitigation Steps

implement tracking of quorum for proposals from the time of creation. During execution, compare the stored quorum with the current quorum to ensure accurate execution of proposals.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L14 https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L102

Vulnerability details

Impact

The protocol may experience a reduction in liquidity/shares and suboptimal trades due to the absence of slippage protection and deadline checks

Proof of Concept

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/DAO.sol#L316

The DAO invokes formPOL to create SALT/USD or USDS/DAI protocol-owned liquidity but does not specify parameters for slippages/minimum liquidity and deadline.

This function then calls

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L146

collateralAndLiquidity.depositLiquidityAndIncreaseShare() with minLiquidityReceived=0 and deadline=block.timestamp i.e neither slippage nor deadline has been implemented properly

Moreover, within the same function, the deadline parameter is discarded before any checks for calling internal _depositLiquidityAndIncreaseShare(). In other words, even if a front-end user attempts to provide a deadline, it won't be taken into account.

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L102

Upon further analysis, it becomes evident that it adds/withdraw liquidity to/from the pool without any slippage and deadline checks, leading to very unfavorable trades

Tools Used

Manual

Recommended Mitigation Steps

Implement slippage and deadline checks appropriately throughout the codebase.

Assessed type

Token-Transfer

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/price_feed/CoreChainlinkFeed.sol#L21

Vulnerability details

Impact

Reserve ratios and pool constraints may break if the WBTC bridge becomes compromised, and WBTC undergoes depegging

Proof of Concept

The chainlink BTC/USD oracle is utilized to price WBTC [https://github.com/code-423n4/2024-01-salty/blob/main/src/price_feed/CoreChainlinkFeed.sol#L21] . WBTC, being a bridged asset, could depeg if the bridge is compromised or fails, causing it to no longer be equivalent to BTC. This scenario would result in all swap functions (essentially the entire protocol) being active against an asset that is now effectively worthless.The protocol continues to value WBTC via BTC/USD, leading to several potential malicious actions, especially since many invariants depend on the WBTC/ETH pool

As the protocol operates smoothly with at least two functioning price feeds,the CoreUniswapFeed UniV3 TWAP safeguards against such asset depegging but other two CoreSaltyFeed and CoreChainlinkFeed will have no effect and hence protocol will keep functioning even in such case

Tools Used

Manual

Recommended Mitigation Steps

Conduct a thorough analysis and implement a robust solution for determining the price of WBTC.

Assessed type

Oracle

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/launch/BootstrapBallot.sol#L69

Vulnerability details

Impact

f the protocol fails to receive enough votes during the initial proposal, all funds sent to the InitialDistribution will become stuck (1000000 ether)

Proof of Concept

https://github.com/code-423n4/2024-01-salty/blob/main/src/launch/BootstrapBallot.sol#L69

The finalizeBallot() function is called for the initial distribution, airdrop, and starting the exchange at the launch time. However, if startExchangeYes <= startExchangeNo, this condition leads to ballotFinalized = true, preventing further calls. There is currently no way to retrieve the funds, as the protocol assumes it will receive startExchangeYes > startExchangeNo. A malicious competitor or attacker could exploit this by funding the attack or employing other methods to ensure the condition holds

The main issue is that the protocol cannot propose for a vote a second time. If it fails to get enough startExchange votes the first time, it will lose all the funds

Tools Used

Manual

Recommended Mitigation Steps

Implement a better approach that allows the protocol to repropose in case it doesn't receive enough votes initially:

1. Provide a period for protocol teams to reevaluate and advertise the protocol after the first failed proposal

2. Implement measures to make reproposing difficult for attackers by increasing the cost of such attacks

Assessed type

DoS