zhoo - The StakingRewards will be sandwich attacked

[sherlock-admin3]

zhoo

Medium

The StakingRewards will be sandwich attacked

Summary

When rewardRate or rewardPerToken is increased, the attacker runs the stake token forward, The attacker gains a reward after the rewardPerToken is increased, and finally stakes the token, the attacker loses nothing and contributes nothing to the protocol, but gains a reward.

Root Cause

1. An attacker can immediately unstake(withdraw) after a stake token:

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/endgame-toolkit/src/synthetix/StakingRewards.sol#L102-L121

    function stake(uint256 amount) public nonReentrant notPaused updateReward(msg.sender) {
        require(amount > 0, "Cannot stake 0");
        _totalSupply = _totalSupply + amount;
        _balances[msg.sender] = _balances[msg.sender] + amount;
        stakingToken.safeTransferFrom(msg.sender, address(this), amount);
        emit Staked(msg.sender, amount);
    }
    function withdraw(uint256 amount) public nonReentrant updateReward(msg.sender) {
        require(amount > 0, "Cannot withdraw 0");
        _totalSupply = _totalSupply - amount;
        _balances[msg.sender] = _balances[msg.sender] - amount;
        stakingToken.safeTransfer(msg.sender, amount);
        emit Withdrawn(msg.sender, amount);
    }

2. The reward is calculated as follows:

    function earned(address account) public view returns (uint256) {
        return (_balances[account] * (rewardPerToken() - userRewardPerTokenPaid[account])) / 1e18 + rewards[account];
    }

The attacker has a stake before the rewardPerToken is increased, and userRewardPerTokenPaid is the old value, so the user can get the reward after the increase.

3. notifyRewardAmount assigns rewards based on time, but this does not prevent sandwich attack: If periodFinish is updated with a long interval, the increment of rewardPerToken after rewardRate is added is: deltaRewardRate = rewardRate * UpdateTimeInterval

After rewardRate is updated, but the interval is shorter, the value of 'rewardPerToken' is increased less, and the attacker can get less reward at this time,

But attackers can wait for the right moment to strike, If lastUpdateTime is not updated after a long time, the attacker finds that another user calls lastUpdateTime(or triggers itself), and the time difference is large and the increment of rewardPerToken is large, the attacker can implement the sandwich attack again.

    //  rewardPerToken += ((lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18) / _totalSupply
    function rewardPerToken() public view returns (uint256) {
        if (_totalSupply == 0) {
            return rewardPerTokenStored;
        }
        return
            rewardPerTokenStored + (((lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18) / _totalSupply);
    }

Internal pre-conditions

The protocol calculates the reward based on the user's _balances.

External pre-conditions

The share of rewards allocated to users increases.

Attack Path

1. The attacker found that the share of rewards increased.
2. Attackers use front-running to stake tokens and increase their balances before the share increases.
3. The share of rewards increases and the attacker gets a reward.
4. The attacker withdrew tokens.
5. An attacker can use flash-loans to attack.

Impact

The attacker loses nothing and contributes nothing to the protocol, but gains a reward.

PoC

Mitigation

Add time lock when extracting revenue.

[telome]

It's hard to understand the reported issue, but it seems to relate to the original Synthetix StakingRewards code, so out of scope.