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

9 stars 5 forks source link

Users can miss out on rebasing credit token rewards #1270

Closed c4-bot-10 closed 4 months ago

c4-bot-10 commented 6 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L181-L182 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L429

Vulnerability details

Impact

The credit token is a rebasing token which increases in quantity in a user's wallet due to the distribute() function as well as other rebasing mechanics. Thus users can expect increase in their owned credit tokens over time without participating in any other parts of the system. However, these rebasing rewards are lost/forfeited by the SurplusGuildMinter and ProfitManager contracts.

The SurplusGuildMinter contract keeps track of the amount of credit tokens deposited by the user at the start of staking. However it does not give the user rebasing rewards on top of that amount which the contract itself gains due to rebases. Thus these rewards are forfeit.

Similarly, the ProfitManager contract keeps some credit tokens which it gives out to gauge weight providers. These rewards are also only based on the profit made by the gauges, and does not include rebase rewards.

So while these two contracts earn rebase rewards in credit tokens, they dont pass them on to the users.

Proof of Concept

The contracts are not designed to handle rebased rewards. This is because they use normal accounting and store the values during deposit. Thus users lose out on the accrued rewards which stay locked in these contracts.

Tools Used

Manual Review

Recommended Mitigation Steps

Build a wrapper for the reward token and use the wrapped version in the contracts. The wrapped version will be a normal fixed value token.

Assessed type

Context

c4-pre-sort commented 6 months ago

0xSorryNotSorry marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as remove high or low quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as primary issue

eswak commented 5 months ago

Acknowledging this, should be marked informational imo, as describe in the issue these contracts are not designed to earn rebasing rewards, if users want to earn the savings rate they have to claim their rewards and hold the tokens in their wallet.

c4-sponsor commented 5 months ago

eswak (sponsor) acknowledged

c4-sponsor commented 5 months ago

eswak marked the issue as disagree with severity

c4-judge commented 5 months ago

Trumpero changed the severity to QA (Quality Assurance)

c4-judge commented 5 months ago

Trumpero marked the issue as grade-b

c4-judge commented 4 months ago

Trumpero marked the issue as grade-c