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

17 stars 11 forks source link

Can stake in the ETH term using the gUSDC credit token #227

Closed c4-bot-2 closed 9 months ago

c4-bot-2 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L136 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L259-L264

Vulnerability details

Impact

There are multiple markets, and users try to augment the weight of a specific term through the SurplusGuildMinter. To achieve this, they utilize the credit token associated with that term. This action enables them to boost the potential debt ceiling for the specific term and also qualify for rewards.

However, an issue arises when users stake in the ETH term using the gUSDC credit token. While this increases the weight of the term in an entirely different market, it prevents the user from receiving any rewards generated in that term. Consequently, some rewards become trapped in that market, inaccessible for any claim.

In the end, the user can reclaim their gUSDC tokens if the ETH term does not incur any losses. I've marked this as a medium because it violates the protocol, hindering the complete distribution of rewards to guild holders, credit holders, and the surplus buffer.

Proof of Concept

There is no check in place when a user attempts to stake in a term through the SurplusGuildMinter to determine whether that term belongs to this market. https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L136

function stake(address term, uint256 amount) external whenNotPaused {
    ProfitManager(profitManager).donateToTermSurplusBuffer(term, amount);
    GuildToken(guild).incrementGauge(term, guildAmount);
}

For instance, a user can stake in an ETH term using a gUSDC credit token, even if that term is not part of the USDC market. The staked gUSDC credits are then directly transferred to the ProfitManager of the USDC market. https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L259-L264

function donateToTermSurplusBuffer(address term, uint256 amount) external {
    CreditToken(credit).transferFrom(msg.sender, address(this), amount);
    uint256 newSurplusBuffer = termSurplusBuffer[term] + amount;     
    termSurplusBuffer[term] = newSurplusBuffer;
}

Clearly, since the ETH term is outside the USDC market, there is no opportunity to utilize these credit tokens in the surplus buffer for this term. This is because the ProfitManager for the USDC market never calls the notifyPnL function with this term. Instead, the ETH term will trigger the notifyPnL function in the ProfitManager for the ETH market.

The gauge weight of this ETH term includes the staked weight using gUSDC credit tokens. When rewards are generated in this term, they should be distributed to all guild holders who contributed to the gauge weight. However, the user who staked using gUSDC cannot receive his rewards. This is due to the fact that the rewards are recorded in the ProfitManager for the ETH market, not the USDC market. Consequently, the rewards for this user remain in the SurplusGuildMinter for the ETH market, and no user can claim these rewards.

In the event of a loss in the ETH term, the user is unable to withdraw gUSDC from the Surplus Buffer for that term in the ProfitManager for the USDC Market because his stake information will be reset. Consequently, the gUSDC credit tokens will be stuck in the ProfitManager for the USDC Market.

The PoC for this is as below:

function testStakeToOtherMarket() public {
        ProfitManager profitManager_1;
        MockERC20 collateral_1;
        CreditToken credit_1;
        GuildToken guild_1;
        RateLimitedMinter rlcm_1;
        AuctionHouse auctionHouse_1;
        LendingTerm term_1;
        SimplePSM psm_1;
        RateLimitedMinter rlgm_1;
        SurplusGuildMinter sgm_1;

        profitManager_1 = new ProfitManager(address(core));
        collateral_1 = new MockERC20();
        credit_1 = new CreditToken(address(core), "name_1", "symbol_1");
        guild_1 = new GuildToken(
            address(core),
            address(profitManager_1)
        );
        rlcm_1 = new RateLimitedMinter(
            address(core) /*_core*/,
            address(credit_1) /*_token*/,
            CoreRoles.RATE_LIMITED_CREDIT_MINTER /*_role*/,
            type(uint256).max /*_maxRateLimitPerSecond*/,
            type(uint128).max /*_rateLimitPerSecond*/,
            type(uint128).max /*_bufferCap*/
        );
        auctionHouse_1 = new AuctionHouse(address(core), 650, 1800);
        term_1 = LendingTerm(Clones.clone(address(new LendingTerm())));
        term_1.initialize(
            address(core),
            LendingTerm.LendingTermReferences({
                profitManager: address(profitManager_1),
                guildToken: address(guild_1),
                auctionHouse: address(auctionHouse_1),
                creditMinter: address(rlcm_1),
                creditToken: address(credit_1)
            }),
            LendingTerm.LendingTermParams({
                collateralToken: address(collateral_1),
                maxDebtPerCollateralToken: 2000e18,
                interestRate: 0.10e18,
                maxDelayBetweenPartialRepay: 63115200, // 2 years
                minPartialRepayPercent: 0.2e18,
                openingFee: 0,
                hardCap: 20_000_000e18
            })
        );
        psm_1 = new SimplePSM(
            address(core),
            address(profitManager_1),
            address(credit_1),
            address(collateral_1)
        );
        rlgm_1 = new RateLimitedMinter(
            address(core), /*_core*/
            address(guild_1), /*_token*/
            CoreRoles.RATE_LIMITED_GUILD_MINTER, /*_role*/
            type(uint256).max, /*_maxRateLimitPerSecond*/
            type(uint128).max, /*_rateLimitPerSecond*/
            type(uint128).max /*_bufferCap*/
        );
        sgm_1 = new SurplusGuildMinter(
            address(core),
            address(profitManager_1),
            address(credit_1),
            address(guild_1),
            address(rlgm_1),
            MINT_RATIO,
            REWARD_RATIO
        );
        vm.startPrank(governor);
        core.grantRole(CoreRoles.CREDIT_MINTER, address(psm_1));
        core.grantRole(CoreRoles.CREDIT_MINTER, address(rlcm_1));
        core.grantRole(CoreRoles.RATE_LIMITED_CREDIT_MINTER, address(term_1));
        core.grantRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(term_1));
        core.grantRole(CoreRoles.GUILD_MINTER, address(rlgm_1));
        core.grantRole(CoreRoles.RATE_LIMITED_GUILD_MINTER, address(sgm_1));
        core.grantRole(CoreRoles.GUILD_SURPLUS_BUFFER_WITHDRAW, address(sgm_1));
        profitManager_1.initializeReferences(address(credit_1), address(guild_1), address(psm_1));
        vm.stopPrank();

        address term_2 = address(2);

        guild_1.setMaxGauges(10);
        // term for USDC market
        guild_1.addGauge(1, address(term_1));
        // term for ETH market
        guild_1.addGauge(2, term_2);

        uint128 X = 100e18;

        // stake
        credit_1.mint(address(this), X);
        credit_1.approve(address(sgm_1), X);
        // We stake in the ETH term using gUSDC.
        sgm_1.stake(term_2, X);
        // Successfully added weight to the ETH term.
        assertGt(guild_1.getUserGaugeWeight(address(sgm_1), term_2), 0); // 200000000000000000000

        uint256 beforeBalance = credit_1.balanceOf(address(this));
        sgm_1.unstake(term_2, X);
        uint256 afterBalance = credit_1.balanceOf(address(this));
        // Successfully unstaked.
        assertEq(afterBalance, beforeBalance + X);
}

Tools Used

Recommended Mitigation Steps

function stake(address term, uint256 amount) external whenNotPaused {
+    require(LendingTerm(term).getReferences().creditToken == credit, "Different Market");
}

Assessed type

Error

c4-pre-sort commented 9 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 9 months ago

0xSorryNotSorry marked the issue as duplicate of #1032

c4-judge commented 8 months ago

Trumpero marked the issue as satisfactory