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

9 stars 5 forks source link

SurplusGuildMinter's operations can be DOS'ed if there exist enough active gauges #1254

Closed c4-bot-8 closed 5 months ago

c4-bot-8 commented 6 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L444

Vulnerability details

Impact

Since operations on a list of gauges (for eg. claimRewards) can be DOS'ed on some chains having lower block gas limits, there is a protection in place called the maxGauges which is currently set to 10.

But the problem is that SurplusGuildMinter is being set to canExceedMaxGauges in the GIP_0 deployment script. This means that the SGM can open positions in any number of gauges. If enough active lending terms (gauges) exist, then users can stake through the SGM into many gauges such that now they get added to the userGauges reference list for the SGM address. This can make the userGauges list really long and cause DOS because of not fitting into the block gas limit.

The userGauges list is being used in ProfitManager.claimRewards function which iterates over the complete list of gauges for an address and claims its rewards from there. This claimRewards function is also an important part of the SurplusGuildMinter.getRewards() function which is basically used in all functionalities of SGM.

The number of gauges in userGauges[SGM address] can be increased very easily because every time a user stakes into a new gauge in SGM, the SGM will have a new gauge added to its list. Thus, this can cause DOS on some chains, which will lead to complete bricking of the Surplus Guild Minter, probably forever.

Proof of Concept

getRewards calls claimRewards and is used in all operations on SGM : https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L239

claimRewards iterates over the whole of userGauges list : https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L444

On incrementGauge, every new gauge is added to the userGauges list : https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/ERC20Gauges.sol#L235

Tools Used

Manual review

Recommended Mitigation Steps

Cant think of a straightforward way to prevent this, maybe have a hard capping on surplus guild minter's maxGauges.

Assessed type

DoS

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 duplicate of #1110

c4-judge commented 5 months ago

Trumpero marked the issue as satisfactory