stake.link
stake.link
DeFiHardhatBridge
27,500 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Stepwise reward distribution will allow people to front run and back run the rewards distribution function to stake for one block using 0 locking duration

happyformerlawyer

Summary

A malicious user can monitor the mempool for when the distributeRewards() function in SDLPoolPrimaryController.sol is called and can front run that call by staking tokens with a 0 locking duration right before the call to distributeRewards() is processed and then back-running by withdrawing his tokens, all in the same block. A user can theoretically do this every time distributeRewards() is called without ever being in the pool longer than one block.

This steals rewards from people who remain in the pool long term and defeats the purpose of the protocol. This is possible because you aren't required to lock your tokens to still get some of the rewards and because rewards begin to accrue right away.

Note: some of the functions discussed are in RewardsPool.sol which is not directly in scope, but several of the contracts that are in scope (e.g., the SDLPool contracts) inherit from RewardsPoolController.sol which, in its _updateRewards() function, calls RewardsPool.sol and those in-scope contracts use the updateRewards() modifier which ultimately calls functions in RewardsPool.sol

Vulnerability Details

This is the _distributeRewards() function in SDLPoolCCIPControllerPrimary.sol which builds a CCIP message that will send the rewards to the appropriate rewards pool on the applicable chain.

function _distributeRewards(
uint64 _destinationChainSelector,
address[] memory _rewardTokens,
uint256[] memory _rewardTokenAmounts
) internal {
address destination = whitelistedDestinations[_destinationChainSelector];
if (destination == address(0)) revert InvalidDestination();
​
uint256 numRewardTokensToTransfer;
for (uint256 i = 0; i < _rewardTokens.length; ++i) {
if (_rewardTokenAmounts[i] != 0) {
numRewardTokensToTransfer++;
}
}
​
if (numRewardTokensToTransfer == 0) return;
​
address[] memory rewardTokens = new address[](numRewardTokensToTransfer);
uint256[] memory rewardTokenAmounts = new uint256[](numRewardTokensToTransfer);
uint256 tokensAdded;
for (uint256 i = 0; i < _rewardTokens.length; ++i) {
if (_rewardTokenAmounts[i] != 0) {
rewardTokens[tokensAdded] = _rewardTokens[i];
rewardTokenAmounts[tokensAdded] = _rewardTokenAmounts[i];
tokensAdded++;
}
}
​
Client.EVM2AnyMessage memory evm2AnyMessage = _buildCCIPMessage(
destination,
0,
rewardTokens,
rewardTokenAmounts,
rewardsExtraArgsByChain[_destinationChainSelector]
);
​
IRouterClient router = IRouterClient(this.getRouter());
uint256 fees = router.getFee(_destinationChainSelector, evm2AnyMessage);
​
if (fees > maxLINKFee) revert FeeExceedsLimit(fees);
bytes32 messageId = router.ccipSend(_destinationChainSelector, evm2AnyMessage);
​
emit DistributeRewards(messageId, _destinationChainSelector, fees);
}

The distributeRewards() function in RewardsPool.sol is called when rewards are distributed and it in turn calls _updateRewardsPerToken() which increments the additional rewards to the rewardPerToken variable

function distributeRewards() public virtual {
require(controller.totalStaked() > 0, "Cannot distribute when nothing is staked");
uint256 toDistribute = token.balanceOf(address(this)) - totalRewards;
totalRewards += toDistribute;
_updateRewardPerToken(toDistribute);
emit DistributeRewards(msg.sender, controller.totalStaked(), toDistribute);
}

The updateReward() function will be called when the malicious user stakes their tokens before the reward distribution is processed, and it will record his userRewardPerTokenPaid as the existing rewardPerToken (right before rewardPerToken is increased by the call to distributeRewards()).

function updateReward(address _account) public virtual {
uint256 newRewards = withdrawableRewards(_account) - userRewards[_account];
if (newRewards > 0) {
userRewards[_account] += newRewards;
}
userRewardPerTokenPaid[_account] = rewardPerToken;
}

The malicious user's rewards are given by the withdrawableRewards() function and they will equal his effective balance (amount staked) times (the now increased rewardPerToken - userRewardPerTokenPaid). His userRewardPerTokenPaid will equal the rewardPerToken before the distribution and rewardPerToken will be larger than that thanks to the new distribution (effected by _updateRewardPerToken()) that happened in the same block right after he staked. He can withdraw same block.

function withdrawableRewards(address _account) public view virtual returns (uint256) {
return
(controller.staked(_account) * (rewardPerToken - userRewardPerTokenPaid[_account])) /
1e18 +
userRewards[_account];
}

Impact

The impact is that the people in the pool long term don't get as many rewards as they should, and the protocol is used in a way that it is not intended to be used. Plus, someone could potentially use a flash loan to do this so they could actually take a very disproportionate amount of the rewards by making their stake very big.

Tools Used

Manual review

Recommendations

Require people to stake for a period of time before rewards begin to accrue. Another thing you could do is incorporate time staking into rewards such that you don't have rewardsPerToken in RewardsPool.sol but rewardsPerTokenPerSecond

Updates

Lead Judging Commences

0kage Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Out of scope
happyformerlawyer Submitter
almost 2 years ago
0kage Lead Judge
almost 2 years ago
0kage Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

min-duration

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.