When a term is offboarded but not cleaned up and then it is onboarded again, an attacker can offboard it freely.

[c4-bot-4]

Lines of code

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

Vulnerability details

Impact

When a term is offboarded but not cleaned up and then it is onboarded again, any user who don't have any weight can propose and offboard it freely without passing supporting process.

Proof of Concept

We say that a term is offboarded.

    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);
    }

Then canOffboard[term] == true.
Because of some reasons, for example issuance not repayed, it remains not cleaned up and then it can be onboarded again.
At this time, any user can propose it in offboarding. Then polls[block.number][term] is smaller than quorum but canOffboard[term] == true.
So any user can offboard it immediately.

    function proposeOffboard(address term) external whenNotPaused {
        require(
            polls[block.number][term] == 0,
            "LendingTermOffboarding: poll exists"
        );
        require(
            block.number > lastPollBlock[term] + POLL_DURATION_BLOCKS,
            "LendingTermOffboarding: poll active"
        );
        // Check that the term is an active gauge
        require(
            GuildToken(guildToken).isGauge(term),
            "LendingTermOffboarding: not an active term"
        );

        polls[block.number][term] = 1; // voting power
        lastPollBlock[term] = block.number;
        emit OffboardSupport(
            block.timestamp,
            term,
            block.number,
            address(0),
            1
        );
    }

Tools Used

Manual Review

Recommended Mitigation Steps

LendingTermOffboarding.sol#proposeOffboard function has to be modified as follows.

    function proposeOffboard(address term) external whenNotPaused {
        require(
            polls[block.number][term] == 0,
            "LendingTermOffboarding: poll exists"
        );
        require(
            block.number > lastPollBlock[term] + POLL_DURATION_BLOCKS,
            "LendingTermOffboarding: poll active"
        );
        // Check that the term is an active gauge
        require(
            GuildToken(guildToken).isGauge(term),
            "LendingTermOffboarding: not an active term"
        );

        polls[block.number][term] = 1; // voting power
        lastPollBlock[term] = block.number;
+       canOffboard[term] = false;

        emit OffboardSupport(
            block.timestamp,
            term,
            block.number,
            address(0),
            1
        );
    }

Assessed type

Error

[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)