Users can receive creditRewards even when they are slashed

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L249-L269

Vulnerability details

Impact

Whenever a term suffers a loss, the users(guild holders) of that term are slashed. Basically, they will not receive anything.
However, users are still able to receive creditRewards even when they are slashed.

In the contract SurplusGuildMinter, the function getRewards() is called whenever a user stakes or unstakes his credit tokens. i.e.
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L249-L269

function getRewards(address user,address term) public
        returns (uint256 lastGaugeLoss, UserStake memory userStake, bool slashed)
    {
 if (_profitIndex == 0) _profitIndex = 1e18;
        if (_userProfitIndex == 0) _userProfitIndex = 1e18;
        uint256 deltaIndex = _profitIndex - _userProfitIndex;     
        if (deltaIndex != 0) {
            uint256 creditReward = (uint256(userStake.guild) * deltaIndex) /
                1e18;
            uint256 guildReward = (creditReward * rewardRatio) / 1e18;   
            if (slashed) {
                guildReward = 0;
            }
            // forward rewards to user
            if (guildReward != 0) {
                RateLimitedMinter(rlgm).mint(user, guildReward);  
                emit GuildReward(block.timestamp, user, guildReward);  
            }
            if (creditReward != 0) {
                CreditToken(credit).transfer(user, creditReward); 
            }

If the user has been slashed, then guildReward are set to zero i.e.

if (slashed) {
guildReward = 0;
}

However, we did not account for creditReward(when users are slashed), therefore, the creditRewards would be sent to the user whenever guildReward function is called i.e.

 if (deltaIndex != 0) {
         uint256 creditReward = (uint256(userStake.guild) * deltaIndex) / 1e18;

if (creditReward != 0) {
    CreditToken(credit).transfer(user, creditReward); 
    }

This is an undesired behavior, and leads to loss of funds as the user is not supposed to receive the creditReward.

Proof of Concept

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L249-L269

Tools Used

Manual Review

Recommended Mitigation Steps

Consider updating the variable creditReward to zero in if the statement. i.e.

if (slashed) {
guildReward = 0;
creditReward = 0;   // DO THIS 
}

Assessed type

Math

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #956

[c4-judge]

Trumpero marked the issue as duplicate of #262

[c4-judge]

Trumpero changed the severity to QA (Quality Assurance)

[c4-judge]

Trumpero marked the issue as grade-b