code-423n4 / 2023-03-mute-findings

2 stars 1 forks source link

`MuteAmplifier.rescueTokens()` should check conditions for fee tokens(token0/token1) as well #33

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-mute/blob/4d8b13add2907b17ac14627cfa04e0c3cc9a2bed/contracts/amplifier/MuteAmplifier.sol#L180

Vulnerability details

Impact

rescueTokens() can be used to withdraw fee tokens without any validations.

As a result, the reward logic would be broken due to the lack of fee tokens.

Proof of Concept

rescueTokens() doesn't validate anything for the fee tokens.

So if some fee tokens were withdrawn, the reward withdrawal logic will revert as the contract doesn't have enough balance.

File: MuteAmplifier.sol
257:         // payout fee0 fee1
258:         if ((fee0 > 0 || fee1 > 0) && fee0 <= totalFees0 && fee1 <= totalFees1) {
259:             address(IMuteSwitchPairDynamic(lpToken).token0()).safeTransfer(msg.sender, fee0);
260:             address(IMuteSwitchPairDynamic(lpToken).token1()).safeTransfer(msg.sender, fee1);
261: 
262:             totalClaimedFees0 = totalClaimedFees0.add(fee0);
263:             totalClaimedFees1 = totalClaimedFees1.add(fee1);
264: 
265:             emit FeePayout(msg.sender, fee0, fee1);
266:         }

Tools Used

Manual Review

Recommended Mitigation Steps

rescueTokens() should validate for the fee tokens as well.

    function rescueTokens(address tokenToRescue, address to, uint256 amount) external virtual onlyOwner nonReentrant {
        address token0 = IMuteSwitchPairDynamic(lpToken).token0();
        address token1 = IMuteSwitchPairDynamic(lpToken).token1();

        ...
        ...
        else if (tokenToRescue == token0) {
            require(amount <= IERC20(token0).balanceOf(address(this)).sub(totalFees0.sub(totalClaimedFees0)),
                "MuteAmplifier::rescueTokens: that token0 belongs to stakers"
            );
        }
        else if (tokenToRescue == token1) {
            require(amount <= IERC20(token1).balanceOf(address(this)).sub(totalFees1.sub(totalClaimedFees1)),
                "MuteAmplifier::rescueTokens: that token1 belongs to stakers"
            );
        }

        IERC20(tokenToRescue).transfer(to, amount);
    }
c4-judge commented 1 year ago

Picodes marked the issue as primary issue

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #18

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory