Ethereum Credit Guild - Varun_05's results

Lines of code

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

Vulnerability details

Impact

Causes users to slashed when they are not causing them to loose their stake and not able to earn rewards.Precisily when a gauge is onboarded again after a gaugeloss then it will always return slashed for every user even if they are not.

Proof of Concept

Below is a get rewards function

  function getRewards(
        address user,
        address term
    )
        public
        returns (
            uint256 lastGaugeLoss, // GuildToken.lastGaugeLoss(term)
            UserStake memory userStake, // stake state after execution of getRewards()
            bool slashed // true if the user has been slashed
        )
    {
        bool updateState;
        lastGaugeLoss = GuildToken(guild).lastGaugeLoss(term);
        if (lastGaugeLoss > uint256(userStake.lastGaugeLoss)) {
            slashed = true;
        }

        // if the user is not staking, do nothing
        userStake = _stakes[user][term];
        if (userStake.stakeTime == 0)
            return (lastGaugeLoss, userStake, slashed);

        // compute CREDIT rewards
        ProfitManager(profitManager).claimRewards(address(this)); // this will update profit indexes
        uint256 _profitIndex = ProfitManager(profitManager)
            .userGaugeProfitIndex(address(this), term);
        uint256 _userProfitIndex = uint256(userStake.profitIndex);

        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);
            }

            // save the updated profitIndex
            userStake.profitIndex = SafeCastLib.safeCastTo160(_profitIndex);
            updateState = true;
        }

        // if a loss occurred while the user was staking, the GuildToken.applyGaugeLoss(address(this))
        // can be called by anyone to slash address(this) and decrement gauge weight etc.
        // The contribution to the surplus buffer is also forfeited.
        if (slashed) {
            emit Unstake(block.timestamp, term, uint256(userStake.credit));
            userStake = UserStake({
                stakeTime: uint48(0),
                lastGaugeLoss: uint48(0),
                profitIndex: uint160(0),
                credit: uint128(0),
                guild: uint128(0)
            });
            updateState = true;
        }

        // store the updated stake, if needed
        if (updateState) {
            _stakes[user][term] = userStake;
        }
    }

As can be seen from the following lines lastGaugeLoss > uint256(userStake.lastGaugeLoss) here userStake.lastGaugeLoss will always return 0 because the original userStake = _stakes[user][term] which is initialised after being checked

 bool updateState;
        lastGaugeLoss = GuildToken(guild).lastGaugeLoss(term);
        if (lastGaugeLoss > uint256(userStake.lastGaugeLoss)) {
            slashed = true;
        }

        // if the user is not staking, do nothing
        userStake = _stakes[user][term];

Now if a gauge is onboarded again then lastGaugeLoss will be non zero and userStake.lastGaugeLoss will be zero therefore every user will be slashed.

Tools Used

Manual Review

Recommended Mitigation Steps

Following changes should be made

bool updateState;
        lastGaugeLoss = GuildToken(guild).lastGaugeLoss(term);
         userStake = _stakes[user][term];
        if (lastGaugeLoss > uint256(userStake.lastGaugeLoss)) {
            slashed = true;
        }

        
       

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

In some cases the debtCeiling function might not return the correct value which can cause more issuance of loans of errors in other function which rely on the debtCeiling function like the applyGaugeLoss function in GuilToken Contract.

Proof of Concept

Following is debtCeiling function

function debtCeiling(
        int256 gaugeWeightDelta
    ) public view returns (uint256) {
        address _guildToken = refs.guildToken; // cached SLOAD
        uint256 gaugeWeight = GuildToken(_guildToken).getGaugeWeight(
            address(this)
        );
        gaugeWeight = uint256(int256(gaugeWeight) + gaugeWeightDelta);
        uint256 gaugeType = GuildToken(_guildToken).gaugeType(address(this));
        uint256 totalWeight = GuildToken(_guildToken).totalTypeWeight(
            gaugeType
        );
        uint256 creditMinterBuffer = RateLimitedMinter(refs.creditMinter)
            .buffer();
        uint256 _hardCap = params.hardCap; // cached SLOAD
        if (gaugeWeight == 0) {
            return 0; // no gauge vote, 0 debt ceiling
        } else if (gaugeWeight == totalWeight) {
            // one gauge, unlimited debt ceiling
            // returns min(hardCap, creditMinterBuffer)
            return
                _hardCap < creditMinterBuffer ? _hardCap : creditMinterBuffer;
        }
        uint256 _issuance = issuance; // cached SLOAD
        uint256 totalBorrowedCredit = ProfitManager(refs.profitManager)
            .totalBorrowedCredit();
        uint256 gaugeWeightTolerance = ProfitManager(refs.profitManager)
            .gaugeWeightTolerance();
        if (totalBorrowedCredit == 0 && gaugeWeight != 0) {
            // first-ever CREDIT mint on a non-zero gauge weight term
            // does not check the relative debt ceilings
            // returns min(hardCap, creditMinterBuffer)
            return
                _hardCap < creditMinterBuffer ? _hardCap : creditMinterBuffer;
        }
        uint256 toleratedGaugeWeight = (gaugeWeight * gaugeWeightTolerance) /
            1e18;
        uint256 debtCeilingBefore = (totalBorrowedCredit *
            toleratedGaugeWeight) / totalWeight;
        if (_issuance >= debtCeilingBefore) {
            return debtCeilingBefore; // no more borrows allowed
        }
        uint256 remainingDebtCeiling = debtCeilingBefore - _issuance; // always >0
        if (toleratedGaugeWeight >= totalWeight) {
            // if the gauge weight is above 100% when we include tolerance,
            // the gauge relative debt ceilings are not constraining.
            return
                _hardCap < creditMinterBuffer ? _hardCap : creditMinterBuffer;
        }
        uint256 otherGaugesWeight = totalWeight - toleratedGaugeWeight; // always >0
        uint256 maxBorrow = (remainingDebtCeiling * totalWeight) /
            otherGaugesWeight;
        uint256 _debtCeiling = _issuance + maxBorrow;
        // return min(creditMinterBuffer, hardCap, debtCeiling)
        if (creditMinterBuffer < _debtCeiling) {
            return creditMinterBuffer;
        }
        if (_hardCap < _debtCeiling) {
            return _hardCap;
        }
        return _debtCeiling;
    }

As can be seen from the comments it should return min(creditMinterBuffer, hardCap, debtCeiling) but currently it misses the scenario when _hardCap <creditMinterBuffer < _debtCeiling then it should return _hardCap but it returns creditMinterBuffer

Similarly when creditMinterBuffer < _hardCap < _debtCeiling

Tools Used

Manual review

Recommended Mitigation Steps

Make the following changes

if (creditMinterBuffer < _debtCeiling) {
            return creditMinterBuffer <_hardCap ?creditMinterBuffer: _hardCap;
        }
        if (_hardCap < _debtCeiling) {
            return _hardCap < creditMinterBuffer ?_hardCap: creditMinterBuffer;
        }

Assessed type

Error