Malicious user can atomically take up to 1% of rewards for all newly whitelisted pools, by using DUST LP

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/DAO.sol#L235-L273 https://github.com/code-423n4/2024-01-salty/blob/main/src/Upkeep.sol#L205-L213 https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/StakingRewards.sol#L182-L206

Vulnerability details

Impact

When pools are first whitelisted, a malicious user can atomically take up to 1% of the SALT emissions rewards for those pools in a single tx using DUST amount of LP. Since bootstrapping rewards per pool is defaulted to 200_000 SALT, and since whitelisting a token means whitelisting both the token-WBTC and token-WETH pool, the user can take up to 2_000 * 2 = 4_000 SALT in a single tx. This can be done by putting at little as 101 wei of both tokens into each pool to create LP, meaning there is no cost to the user.

Proof of Concept

When a ballot for whitelisting a new token in the DAO contract can be finalized, a malicious user will craft a single transaction:

1. call DAO:finalizeBallot which triggers _finalizeTokenWhitelisting. As part of the logic of this function, the following code is run:

   poolsConfig.whitelistPool( pools,  IERC20(ballot.address1), exchangeConfig.wbtc() );
   poolsConfig.whitelistPool( pools,  IERC20(ballot.address1), exchangeConfig.weth() );
   ...
   liquidityRewardsEmitter.addSALTRewards( addedRewards );

   The RewardsEmitter:addSALTRewards function then updates the pending rewards for both these whitelisted pools:

   pendingRewards[ addedReward.poolID ] += amountToAdd;

2. As both the token-WBTC and token-WETH pools are now whitelisted, add liquidity for both pools equal to 101 wei of both tokens. This passes the check that added token amounts are > DUST.

3. Call Upkeep:performUpkeep, which as part of its flow will trigger Upkeep:step8, which has the following logic:

   uint256 timeSinceLastUpkeep = block.timestamp - lastUpkeepTimeRewardsEmitters;
   ...
   saltRewards.liquidityRewardsEmitter().performUpkeep(timeSinceLastUpkeep);

   This in turn triggers RewardsEmitter:performUpkeep, which loops over all whitelisted PoolIDs with the following logic:

   
   ...
   if ( timeSinceLastUpkeep >= MAX_TIME_SINCE_LAST_UPKEEP )
   timeSinceLastUpkeep = MAX_TIME_SINCE_LAST_UPKEEP;

// These are the AddedRewards that will be sent to the specified StakingRewards contract AddedReward[] memory addedRewards = new AddedReward;

// Rewards to emit = pendingRewards timeSinceLastUpkeep rewardsEmitterDailyPercent / oneDay uint256 numeratorMult = timeSinceLastUpkeep rewardsConfig.rewardsEmitterDailyPercentTimes1000(); uint256 denominatorMult = 1 days 100000; // simplification of numberSecondsInOneDay (100 percent) 1000

uint256 sum = 0; for( uint256 i = 0; i < poolIDs.length; i++ ) { bytes32 poolID = poolIDs[i];

// Each pool will send a percentage of the pending rewards based on the time elapsed since the last send @> uint256 amountToAddForPool = ( pendingRewards[poolID] * numeratorMult ) / denominatorMult; ... addedRewards[i] = AddedReward( poolID, amountToAddForPool ); }

@> stakingRewards.addSALTRewards( addedRewards ); }

Ultimately it will call `stakingRewards.addSALTRewards(..)` with the addedRewards being as much as 1% of the emissions for that pool (e.g. 2_000 SALT). [`StakingRewards:addSALTRewards`](https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/StakingRewards.sol#L182-L206) then updates the rewards for those new pools by updated the `totalRewards` mapping:
```solidity
uint256 amountToAdd = addedReward.amountToAdd;
totalRewards[ poolID ] += amountToAdd;

4. User will call StakingRewards:claimAllRewards to withdraw all of these newly added tokens.

Tools Used

Manual review

Recommended Mitigation Steps

SALT emissions should not be sent to newly whitelisted pools right away, rather there should be a waiting period to allow for LPs to enter the pool, allowing for fairer reward distribution.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #614

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 3 (High Risk)