code423n4 commented 12 months ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L418-L425

Vulnerability details

Impact

There can only be one emissionToken per market. If two different users both want to do campaigns with the same token, even if at different times, they share the same "pool". This could either be misused by a user, to intentionally not fund the campaign stealing funds from the other. Or reward claims from one campaign spill over to the next one.

Proof of Concept

Bob requests a campaign where they emit USDC for a market. They then never actually add any USDC but let the rewards accumulate.

Then, after Bobs "campaign" has ended, Alice also wants to do a campaign in USDC. She gets ownership of this configuration and configure their new endTime and speed. The issue is that Bobs previous accumulated rewards are still there and can drain Alice new campaign.

Test in MultiRewardDistributor.t.sol, MultiRewardDistributorCommonUnitTest:

    function testNewOwnerTakesOverEmissionToken() public {
        uint256 startTime = 1678340000;
        vm.warp(startTime);

        // some setup from `createDistributorWithRoundValuesAndConfig`
        MultiRewardDistributor distributor = new MultiRewardDistributor();
        bytes memory initdata = abi.encodeWithSignature("initialize(address,address)", address(comptroller), address(this));
        TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(address(distributor), proxyAdmin, initdata);

        /// wire proxy up
        distributor = MultiRewardDistributor(address(proxy));
        comptroller._setRewardDistributor(distributor);

        distributor._addEmissionConfig(
            mToken,
            address(this),
            address(emissionToken),
            0.1e18,
            0,
            block.timestamp + 10
        );

        faucetToken.allocateTo(address(this), 1e18);
        faucetToken.approve(address(mToken), 1e18);

        // user mints but no tokens added in emissions
        mToken.mint(1e18);
        assertEq(MTokenInterface(mToken).totalSupply(), 1e18);

        // time passes
        vm.warp(block.timestamp + 20);

        // initial user withdraws
        mToken.redeem(1e18);

        // another user uses same token for a new campaign
        address alice = address(0x1111);
        distributor._updateOwner(mToken,address(emissionToken),alice);

        vm.prank(alice);
        distributor._updateEndTime(mToken,address(emissionToken),block.timestamp + 10);

        emissionToken.allocateTo(address(distributor), 1e18);

        vm.warp(block.timestamp + 1 days);

        // original user can claim second campaigns funds without having participated
        comptroller.claimReward();
        assertEq(emissionToken.balanceOf(address(this)), 1e18);
    }

This PoC is a bit handwavey but it shows that the emission "pool" is shared across all users of that token. Hence, in practice, an emissionToken cannot be reused. As soon as someone uses USDC or WETH as a reward token, they can never be used again in the same market.

Tools Used

Manual audit

Recommended Mitigation Steps

Consider adding an ability to remove an emissionConfig. Or add more bookkeeping which owner adds which funds and can only extract rewards for their own funds.