Re-onboarded term can be offboarded instantly

[c4-bot-5]

Lines of code

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

Vulnerability details

Impact

There can be a situation where an offboarded term has been re-onboarded without a cleanup, that term can be offboarded instantly, without a 7 day vote.

Proof of Concept

In order to offboard a term users need to start a 7 day voting in the LendingTermOffboarding.sol, after it succeeded we set canOffboard[term] = true
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/LendingTermOffboarding.sol#L139
and anyone can call offboard function to remove the gauge from the active list
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/LendingTermOffboarding.sol#L153

    function offboard(address term) external whenNotPaused {
        require(canOffboard[term], "LendingTermOffboarding: quorum not met");

        // update protocol config
        // this will revert if the term has already been offboarded
        // through another mean.
        GuildToken(guildToken).removeGauge(term);

        // pause psm redemptions
        if (
            nOffboardingsInProgress++ == 0 &&
            !SimplePSM(psm).redemptionsPaused()
        ) {
            SimplePSM(psm).setRedemptionsPaused(true);
        }

        emit Offboard(block.timestamp, term);
    }

Later this gauge can be re-onboarded with the LendingTermOnboarding.sol, unfortunately canOffboard flag isn't reset for the re-onboarded term and it can be offboarded again by anyone without a vote. To set this flag to false we need to execute a cleanup function
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/LendingTermOffboarding.sol#L197
but this requirement indicates that it is not necessary to call this function if we plan to re-onboard the term
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/LendingTermOffboarding.sol#L181-L184
It is worth noting that the nOffboardingsInProgress counter is also not updated, which can create problems with following requirement in the future
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/LendingTermOffboarding.sol#L191-L195

Check this test case for LendingTermOffboarding.t.sol

    function testInstantOffboardAfterReonboard() public {
        // prepare (1)
        guild.mint(bob, _QUORUM);
        vm.startPrank(bob);
        guild.delegate(bob);
        uint256 snapshotBlock = block.number;
        offboarder.proposeOffboard(address(term));
        vm.roll(block.number + 1);
        vm.warp(block.timestamp + 13);
        offboarder.supportOffboard(snapshotBlock, address(term));
        offboarder.offboard(address(term));
        vm.stopPrank();

        // re-onboard
        guild.addGauge(1, address(term));

        // offboard without voting
        offboarder.offboard(address(term));
    }

Tools Used

Foundry

Recommended Mitigation Steps

Set canOffboard[term] to false and decrement nOffboardingsInProgress if the gauge was previously removed

Assessed type

Governance

[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