Ethereum Credit Guild - almurhasan's results

Lines of code

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

Vulnerability details

Impact

when the guild token is non transferable, there will always be less reward for the guild holders and if guild tokens become transferable, there may remain 0 rewards for the guild holders.

Proof of Concept

1. Let assume total guild tokens = 416(alice has 200,bob has 50 and john has 166 guild tokens).there is only one gauge which has 250 weight(alice has given 200 weight,bob has given 50 weight,john has not given any weight to the gauge). creditSplit =50%, guildSplit = 50%,surplusBufferSplit = 0%,otherSplit = 0%.
2. Now there are 200 rewards for the gauge,50% i.e 100 goes for creditSplit and other 100 goes to alice and bob for guild voting.
3. Now see the code, if (_gaugeWeight != 0) { uint256 _gaugeProfitIndex = gaugeProfitIndex[gauge]; if (_gaugeProfitIndex == 0) { _gaugeProfitIndex = 1e18; } gaugeProfitIndex[gauge] = _gaugeProfitIndex + (amountForGuild * 1e18) / _gaugeWeight; }

so gaugeProfitIndex[gauge] = 1e18+(100e18*1e18)/250e18 = 1e18+0.4e18 =1.4e18. 4. Alice calls for rewards and function claimGaugeRewards is called, see the function claimGaugeRewards code, function claimGaugeRewards( address user, address gauge ) public returns (uint256 creditEarned) { uint256 _userGaugeWeight = uint256( GuildToken(guild).getUserGaugeWeight(user, gauge) ); if (_userGaugeWeight == 0) { return 0; } uint256 _gaugeProfitIndex = gaugeProfitIndex[gauge]; uint256 _userGaugeProfitIndex = userGaugeProfitIndex[user][gauge]; if (_gaugeProfitIndex == 0) { _gaugeProfitIndex = 1e18; } if (_userGaugeProfitIndex == 0) { _userGaugeProfitIndex = 1e18; } uint256 deltaIndex = _gaugeProfitIndex - _userGaugeProfitIndex; if (deltaIndex != 0) { creditEarned = (_userGaugeWeight * deltaIndex) / 1e18; userGaugeProfitIndex[user][gauge] = _gaugeProfitIndex; } if (creditEarned != 0) { emit ClaimRewards(block.timestamp, user, gauge, creditEarned); CreditToken(credit).transfer(user, creditEarned); } }

as initially userGaugeProfitIndex[alice][gauge] = 0,so userGaugeProfitIndex[alice][gauge] sets to 1e18. gaugeProfitIndex[gauge] = 1.4e18. So deltaIndex = 1.4e18-1e18 = 0.4e18.alice’s creditearned = (200e180.4e18)/1e18 = 80e18, similarly bob’s creditearned = (50e180.4e18)/1e18 = 20e18.now userGaugeProfitIndex[alice][gauge] = 1.4e18, userGaugeProfitIndex[bob][gauge]= 1.4e18. 5. Now there is 400 rewards, 50% i.e 200 goes for creditSplit and other 200 goes to alice and bob for guild voting. gaugeProfitIndex[gauge] = 1.4e18+(200e18*1e18)/250e18 = 1e18+0.8e18 =2.2e18. 6. Alice and bob before claiming the reward , john increase(add) 166 weight to the gauge,see the code, function _incrementGaugeWeight( address user, address gauge, uint256 weight ) internal override { uint256 _lastGaugeLoss = lastGaugeLoss[gauge]; uint256 _lastGaugeLossApplied = lastGaugeLossApplied[gauge][user]; if (getUserGaugeWeight[user][gauge] == 0) { lastGaugeLossApplied[gauge][user] = block.timestamp; } else { require( _lastGaugeLossApplied >= _lastGaugeLoss, "GuildToken: pending loss" ); }

    ProfitManager(profitManager).claimGaugeRewards(user, gauge);

    super._incrementGaugeWeight(user, gauge, weight);
}

function _incrementGaugeWeight is called which call ProfitManager(profitManager).claimGaugeRewards(john, gauge), see claimGaugeRewards function , as GuildToken(guild).getUserGaugeWeight(john, gauge) = 0, creditEarned return 0 and john’s ProfitIndex is not updated(this is the attack vector).now john’s weight to the gauge is 160 i.e GuildToken(guild).getUserGaugeWeight(john, gauge) = 166. 7. Alice and bob before claiming the reward, John calls the function claimGaugeRewards, gaugeProfitIndex[gauge] = 2.2e18. as initially userGaugeProfitIndex[alice][gauge] = 0,so userGaugeProfitIndex[alice][gauge] sets to 1e18.So deltaIndex = 2.2e18-1e18 = 1.2e18. john’s creditearned = (166e18*1.2e18)/1e18 = 199.2e18. Now alice and bob have no rewards to claim. 8. Alice and bob before claiming the reward,john can claim the rewards by calling two functions( function incrementGauge ,function claimGaugeRewards) in a single transaction by creating a smart contract.When guild token will be transferable , attacker/john can transfer guild token to another address and attacker can always steal rewards of other guild holders.

Tools Used

manual review

Recommended Mitigation Steps

when the function incrementGauge is called, if caller weight is 0, set userGaugeProfitIndex[caller][gauge] to gaugeProfitIndex[gauge] in function claimGaugeRewards.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

borrower will get back collateral but loss willbe applied for the gauge.

Proof of Concept

1. Let assume bob’s borrowamount = 20000e18, bob’s collateral amount = 15e18.lending term interest rate = 0.10e18. After 1 year bob’s interest = 2000e18 , bob’s loan debt = 22000e18(assume loan.borrowCreditMultiplier = 1e18, present creditMultiplier = 1e18).

2. The lendingterm is offboarded , bob’s loanid is called for auction , now bob’s loan.calldebt = 22000e18.

3. Let assume _AUCTION_DURATION = 30 minutes, _MIDPOINT = 10 minutes,50% of auction first phase has gone and bidder bids, collateralReceived = 7.5e18,creditAsked = 22_000e18, bob gets back 7.5e18 collateral token.

4. See the function onBid, uint256 creditMultiplier = ProfitManager(refs.profitManager) .creditMultiplier(); uint256 borrowAmount = loans[loanId].borrowAmount; uint256 principal = (borrowAmount * loans[loanId].borrowCreditMultiplier) / creditMultiplier; int256 pnl; uint256 interest; if (creditFromBidder >= principal) { interest = creditFromBidder - principal; pnl = int256(interest); } else { pnl = int256(creditFromBidder) - int256(principal); principal = creditFromBidder; require( collateralToBorrower == 0, "LendingTerm: invalid collateral movement" ); } let assume now creditmultiplier is 0.9e18. So principle = (20000e18*1e18)/0.9e18 = 22222e18. As creditfrombidder is less than principle , interest will not be applied, loss will be applied which is unfair because borrower will get back collateral but loss willbe applied for the gauge.

Tools Used

manual review

Recommended Mitigation Steps

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

guild holders can get unfairly more rewards than other guild holders.

Proof of Concept

1. Let assume, total guild = 500(alice has 250 and bob has 250 weight). There are two gauges. Gauge1 has 250 weights(where alice’s weight 200,bob’s weight 50). Gauge2 has 200 weights(where alice’s weight is 50,bob’s weight 200). creditSplit =50%, guildSplit = 50%,surplusBufferSplit = 0%,otherSplit = 0%.
2. Now there are 200 rewards for the gauge1, 50% i.e 100 goes for creditSplit and other 100 goes to alice and bob for guild voting.alice should get 80 reward and bob should get 20 reward.
3. Now see the code, if (_gaugeWeight != 0) { uint256 _gaugeProfitIndex = gaugeProfitIndex[gauge]; if (_gaugeProfitIndex == 0) { _gaugeProfitIndex = 1e18; } gaugeProfitIndex[gauge] = _gaugeProfitIndex + (amountForGuild * 1e18) / _gaugeWeight; }

So gaugeProfitIndex[gauge] = 1e18+(100e18*1e18)/250e18 = 1e18+0.4e18 =1.4e18.

4. see the code, function _decrementGaugeWeight( address user, address gauge, uint256 weight ) internal override { uint256 _lastGaugeLoss = lastGaugeLoss[gauge]; uint256 _lastGaugeLossApplied = lastGaugeLossApplied[gauge][user]; require( _lastGaugeLossApplied >= _lastGaugeLoss, "GuildToken: pending loss" );

    // update the user profit index and claim rewards
    ProfitManager(profitManager).claimGaugeRewards(user, gauge);
   
    // check if gauge is currently using its allocated debt ceiling.
    // To decrement gauge weight, guild holders might have to call loans if the debt ceiling is used.
    uint256 issuance = LendingTerm(gauge).issuance();
    if (issuance != 0) {
        uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
        require(
            issuance <= debtCeilingAfterDecrement,
            "GuildToken: debt ceiling used"
        );
    }
   
    super._decrementGaugeWeight(user, gauge, weight);
   

   }

Bob is watching gauge1 100 profit and frontrun the alice’s claim reward by decreasing gauge2 weights(bob decreases 200 weights ,assume that debt ceiling is not reached) and increasing gauge1 weights(bob increases/add 200 weight).So now bob’s GuildToken(guild).getUserGaugeWeight(bob, gauge1) = 250. 5. see the function claimGaugeRewards function claimGaugeRewards( address user, address gauge ) public returns (uint256 creditEarned) { uint256 _userGaugeWeight = uint256( GuildToken(guild).getUserGaugeWeight(user, gauge) ); if (_userGaugeWeight == 0) { return 0; } uint256 _gaugeProfitIndex = gaugeProfitIndex[gauge]; uint256 _userGaugeProfitIndex = userGaugeProfitIndex[user][gauge]; if (_gaugeProfitIndex == 0) { _gaugeProfitIndex = 1e18; } if (_userGaugeProfitIndex == 0) { _userGaugeProfitIndex = 1e18; } uint256 deltaIndex = _gaugeProfitIndex - _userGaugeProfitIndex; if (deltaIndex != 0) { creditEarned = (_userGaugeWeight * deltaIndex) / 1e18; userGaugeProfitIndex[user][gauge] = _gaugeProfitIndex; } if (creditEarned != 0) { emit ClaimRewards(block.timestamp, user, gauge, creditEarned); CreditToken(credit).transfer(user, creditEarned); } }

Now bob’s creditearned = (250e18*0.4e18)/1e18 = 100e18 . There is no reward for alice to claim. Bob can apply the same process for gauge2 profit by watching profit transaction in the mempool.

Tools Used

manual review

Recommended Mitigation Steps

Create time based reward mechanism i.e per second rewards for the guild holders.

Assessed type

Other