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

17 stars 11 forks source link

SurplusGuildMinter contract might be DoS because function `getRewards` claims rewards from all active lending terms #1140

Closed c4-bot-2 closed 8 months ago

c4-bot-2 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/SurplusGuildMinter.sol#L141

Vulnerability details

Impact

The SurplusGuildMinter.claimRewards function loops and claims all rewards from active lending terms delegated by this contract. Consequently, this function can run out of gas and revert, potentially causing a DoS state on other functions of SurplusGuildMinter. This situation may occur when the market has numerous lending terms, and users stake in many of them.

Proof of Concept

In the SurplusGuildMinter contract, getRewards function is used to claim and transfer rewards from a lending term (gauge) to the user. However, it calls the claimRewards function from the ProfitManager contract, which loops and claims rewards from all delegated lending terms of this contract. That action is unnecessary because it only needs to claim the reward of a specific term, resulting in the risk of reverting due to running out of gas for this function.

function getRewards(
    address user,
    address term
)...
{
    ...
    ProfitManager(profitManager).claimRewards(address(this));
    ...
}

function claimRewards(
    address user
) external returns (uint256 creditEarned) {
    address[] memory gauges = GuildToken(guild).userGauges(user);
    for (uint256 i = 0; i < gauges.length; ) {
        creditEarned += claimGaugeRewards(user, gauges[i]);
        unchecked {
            ++i;
        }
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

getRewards function should call claimGaugeRewards function instead of claimRewards:

ProfitManager(profitManager).claimGaugeRewards(address(this), term);

Assessed type

DoS

c4-pre-sort commented 8 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 8 months ago

0xSorryNotSorry marked the issue as duplicate of #1110

c4-judge commented 8 months ago

Trumpero marked the issue as satisfactory