code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-mute/blob/main/contracts/amplifier/MuteAmplifier.sol#L185-L191

Vulnerability details

Proof of Concept

The logic for RescueTokens doesn't take into account the reward remainders.

I wanted to write a POC but I'm in a bit of a time crunch. So, imagine the following situation: totalRewards = 100, and staker A, B (the only stakers) staked for the first and second half (respectively) of the staking duration with multiplier 1/2.

Remainder and reward for staker A should both be 25.

        reward = lpTokenOut.mul(_totalWeight.sub(_userWeighted[account])).div(calculateMultiplier(account, true));
        // max possible rewards
        remainder = lpTokenOut.mul(_totalWeight.sub(_userWeighted[account])).div(10**18);
        // calculate left over rewards
        remainder = remainder.sub(reward);

https://github.com/code-423n4/2023-03-mute/blob/main/contracts/amplifier/MuteAmplifier.sol#L371-L375

When A withdraws, the remainder gets added to totalReclaimed and transferred to the treasury, while the reward gets added to totalClaimedRewards and locked in dMute.

        if(remainder > 0){
          totalReclaimed = totalReclaimed.add(remainder);
          IERC20(muteToken).transfer(treasury, remainder);
        }
        // payout rewards
        if (reward > 0) {
            uint256 week_time = 60 * 60 * 24 * 7;
            IDMute(dToken).LockTo(reward, week_time ,msg.sender);

            userClaimedRewards[msg.sender] = userClaimedRewards[msg.sender].add(
                reward
            );
            totalClaimedRewards = totalClaimedRewards.add(reward);

            emit Payout(msg.sender, reward, remainder);
        }

https://github.com/code-423n4/2023-03-mute/blob/main/contracts/amplifier/MuteAmplifier.sol#L240-L255

So, after A withdraws, the mute balance of the contract is 50, totalRewards is still 100, and totalClaimedRewards is 25. Assume that 20 mute tokens got transferred to the contract by mistake and need to be rescued. Observe as long as staker B doesn't withdraw (totalStakers > 0), rescueTokens for mute will fail due to arithmetic underflow (balance - (totalRewards - totalClaimedRewards) = 70 - (100-25) = -5 < 0) :

        else if (tokenToRescue == muteToken) {
            if (totalStakers > 0) {
                require(amount <= IERC20(muteToken).balanceOf(address(this)).sub(totalRewards.sub(totalClaimedRewards)),
                    "MuteAmplifier::rescueTokens: that muteToken belongs to stakers"
                );
            }
        }

https://github.com/code-423n4/2023-03-mute/blob/main/contracts/amplifier/MuteAmplifier.sol#L185-L191

Impact

RescueTokens cannot necessarily rescue all the mute tokens.

Tools Used

Manual Review

Recommended Mitigation Steps

https://github.com/code-423n4/2023-03-mute/blob/main/contracts/amplifier/MuteAmplifier.sol#L187 Instead of ^, RescueTokens should check amount <= balance - (totalRewards - totalClaimedRewards - totalReclaimed)

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #32