If a term is re-onboarded before cleanup, SimplePSM::redeem to redeem would be DoS-ed and funds would be locked

[c4-bot-1]

Lines of code

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

Vulnerability details

Impact

To offboard a term GUILD holders should agree on that. After a offboard is accepted, the gauge is removed from active gauges list and redemption inside SimplePSM is paused until all loans are paid. To unpause the redemption, LendingTermOffboarding::cleanup should be called. But we can note that it is a valid scenario to re-onboard a term, before all conditions for cleanup are met. But lets examine what would be the consequences from such an action.

1. When offboard is called nOffboardingsInProgress is incremented by 1 and SimplePSM redemption will be paused as long as nOffboardingsInProgress > 0

2. But if a term is re-onboarded, before all his loans has been repaid, or nobody has called cleanup function we can notice another concern:

   2.1 To offboard a term we need canOffboard[term] to be true, which is set back to false inside cleanup function, which has never been called

   2.2 This means that after re-onboarding a single person can offboard it again by simply calling offboard

   2.3 Which would lead to the worst impact, which is incrementing nOffboardingsInProgress again for the same lending term. This means that now it is impossible to set nOffboardingsInProgress back to 0, because cleanup can be called only once for this term, which will decrement progress variable by only 1. The result is constantly paused SimplePSM and blocked funds for stakers.

   NOTE there is a way for community to vote on unpausing the PSM, but this would take a lot of time, during which all PSM functionalities (mint/redeem) would be blocked and even after it’s unblocking, when another term is off-boarded, we again enter in long pause, which is only changeable after long GOV vote and Timelock waiting period.

• Impact is inconsistencies between important state variables inside LendingTermOffboarding and blocked functionality and funds of SimplePSM

Proof of Concept

I have provided instructions in the following gist on how and where to run the coded PoC.

Tools Used

Manual Review
Foundry

Recommended Mitigation Steps

• One solution is to reset canOffboard for the term, which is being re-onboarded, when this happens. The implementation may be to implement a logic, which would check canOffboard when a proposal execute is called and based on that to remove it and decrement pending offboardings, or do nothing

Assessed type

DoS

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #1147

[c4-judge]

Trumpero marked the issue as duplicate of #1141

[c4-judge]

Trumpero marked the issue as satisfactory

[c4-judge]

Trumpero changed the severity to 2 (Med Risk)