Ethereum Credit Guild - rvierdiiev's results

Lines of code

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

Vulnerability details

Proof of Concept

LendingTermOffboarding contract is used to offboard gauge. Anyone can create proposal for voting and 1 week should pass till last voting.

Then user have 1 week to vote. In case if proposal receives more than quorum, then canOffboard[term] is set to true, which then allows anyone to call offboard and cleanup functions. After cleanup is done, then canOffboard[term] is set to false, so new voting is needed in case if term will be onboarded again.

But in case if time from proposal creation to get enough amount of votes and execution of offboard and cleanup functions is less than 1 week, then anyone can call supportOffboard again which will make canOffboard[term] to be true again.

As result in case if term will be onboarded again later, then anyone will be able to offboard it immediately.

Example:

• proposal to offboard term X was created.
• after 2 days it reaches quorum and is executed
• 3 days later all loans were settled and cleanup was called
• on day 6 someone votes for offboarding again and makes canOffboard[term] to be true
• after 2 months term X was onboarded again
• it was offboarded immediately by LendingTermOffboarding without new voting

Impact

Users can reuse voting to skip next proposal voting.

Tools Used

VsCode

Recommended Mitigation Steps

You can have mapping like block:offboarded and in case if quorum is reached for block, then then set as true and do not allow further voting.

Assessed type

Error

Lines of code

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

Vulnerability details

Proof of Concept

GUILD token holder vote for a gauges. When they increase voting power of a gauge then bigger amount of debt is allowed to be issued by that gauge. In case if user would like to decrement his votes to the gauge, then it's checked that gauge's issuance is smaller than debt ceiling without user's votes. This is done to now allow voters to unvote just before a gauge loss and avoid to be slashed.

Now let's look into LendingTerm(gauge).debtCeiling to see how it calculates debt ceiling. In case if gauge is the only gauge of the type, then it's allowed to borrow min(hardCap, creditMinterBuffer). The same is allowed, when tolerated weight of gauge is bigger than 100%. Also when we found correct ceiling for the gauge that is not only of same type, we also cap it with buffer size.

Pls, note that min(hardCap, creditMinterBuffer) is used, which means that usage of buffer decreases allowed amount. So suppose that we have the only gauge of the type with buffer that allows to mint 10_000 tokens at the moment and issuance of 20_000 tokens. As result when user would like to decrement votes from gauge, then debtCeiling will return current buffer amount which is 10_000 and thus the check will revert.

Impact

User can't decrement votes from gauge in some cases

Tools Used

VsCode

Recommended Mitigation Steps

I don't see the need of rate limiter buffer check here. This is because if buffer doesn't have enough balance to mint, then it will revert on borrow and will not allow bigger amount to be minted. However debt ceiling in reality doesn't depend on rate limiter at all. And as i showed it will not allow users to decrement gauge weight some times.

Assessed type

Error

Lines of code

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

Vulnerability details

Proof of Concept

In case if bad debt has occurred in the term, then it's is socialized over all holders of credit token. As you can see CreditToken(_credit).totalSupply function is used to get current supply of credit token.

CreditToken is rebase token and all accrued interests are distributed during the next 30 days. And its totalSupply function will return total supply on current moment. For example if profit of 3000 tokens were distributed 10 days ago, then totalSupply will add extra 1000 tokens to the supply.

Also ERC20RebaseDistributor has targetTotalSupply function and this function will return us 3000 extra tokens even if distribution period hasn't finished yet.

So the problem is that ProfitManager distributes bad debt to not all tokens in the system and after they all will be redeemed then bigger amount than a loss will be taken from token holders.

Example(just random numbers to make calculations simple):

• gUSDC market
• creditMultiplier is 1, so user provides 1 usdc and get 1 gUSDC
• totalSupply is 1000 and 100 more tokens are going to be distributed in next 30 days, so totalSupply will be 1100 in 30 days
• 100 token loss occurs(100 gUSDC = 100 usdc) right after ERC20RebaseDistributor.distribute is called, which makes multiplier to be 0.9(1000 - 100 / 1000)
• after 30 days we have 1100 tokens in supply and all users redeem with multiplier 0.9, so they are able to get 990 usdc back, but extra 10 usdc were left in the PSM contract
• correct multiplier should be 0.909(1100 - 100 / 1100), in this case debt will be distributed correctly

Impact

Debt is distributed incorrectly, which makes loss for credit token holders

Tools Used

VsCode

Recommended Mitigation Steps

Use targetTotalSupply to get correct token supply. uint256 creditTotalSupply = CreditToken(_credit).targetTotalSupply();

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/ERC20Gauges.sol#L435

Vulnerability details

Proof of Concept

In case if term is taking loss, then it will be offboarded, which will remove gauge and then allows to auction all loans. In case if auction will create a loss, then profit manager will be notified and gauge will be notified about loss. This means that gauge voters will be slashed and that they will not be able to withdraw those votes.

So in case if gauge voter will notice, that a loss will occur, then he would like to withdraw votes from a gauge. That's why GUILD token has check that current issuance of gauge should be smaller than debtCeiling after user will withdraw his votes from gauge.

When gauge is removed, then it's votes are not removed, only type weight is decreased with gauge's votes. This actually means that now debt ceiling has increased for the gauge, because total votes were decreased.

This creates 2 problems:

• Debt ceiling is increasing for the term, which means that some gauge voters can now remove votes from the gauge to avoid slashing. For example if 2 gauges were only and removed gauge had more votes than other, then after remove debtCeiling function will think that gauge has more than 100% of votes and thus will allow maximum ceiling, so voters will be able to remove some votes. Also in such scenario user can withdraw delta amount(gaugeVotes - totalTypeVotes), as then this condition will be met.

• Another problem that can occur is when gauge is the only of the type, then after offboarding type weight will be 0, but gauge weight will not be 0. And as result function will revert, which will not allow users to remove votes in case if loss didn't happen after offboarding.

Impact

Some gauge voters can avoid slashing after offboarding. Unvoting can be stucked until gauge will not be added again.

Tools Used

VsCode

Recommended Mitigation Steps

You can query if gauge is deprecated. In case if yes, then inside debtCeiling function you can increase type weight with gauge weight to get correct weight ratio for the gauge.

Assessed type

Error