LendingTermOffboarding.supportOffboard function can be called to change offboarded term's canOffboard back to true

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/e3d3e581e0e51a9ecf3a5a0c4e4bd4af32552fc0/src/governance/LendingTermOffboarding.sol#L116-L148
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/e3d3e581e0e51a9ecf3a5a0c4e4bd4af32552fc0/src/governance/LendingTermOffboarding.sol#L153-L170
https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/e3d3e581e0e51a9ecf3a5a0c4e4bd4af32552fc0/src/governance/LendingTermOffboarding.sol#L175-L199

Vulnerability details

Impact

After an offboarding proposal gains enough votes through voters' LendingTermOffboarding.supportOffboard function calls, which can be before POLL_DURATION_BLOCKS after snapshotBlock is reached, canOffboard[term] for the corresponding term changes to true.

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/e3d3e581e0e51a9ecf3a5a0c4e4bd4af32552fc0/src/governance/LendingTermOffboarding.sol#L116-L148

    function supportOffboard(
        uint256 snapshotBlock,
        address term
    ) external whenNotPaused {
        require(
            block.number <= snapshotBlock + POLL_DURATION_BLOCKS,
            "LendingTermOffboarding: poll expired"
        );
        uint256 _weight = polls[snapshotBlock][term];
        require(_weight != 0, "LendingTermOffboarding: poll not found");
        uint256 userWeight = GuildToken(guildToken).getPastVotes(
            msg.sender,
            snapshotBlock
        );
        require(userWeight != 0, "LendingTermOffboarding: zero weight");
        require(
            userPollVotes[msg.sender][snapshotBlock][term] == 0,
            "LendingTermOffboarding: already voted"
        );

        userPollVotes[msg.sender][snapshotBlock][term] = userWeight;
        polls[snapshotBlock][term] = _weight + userWeight;
        if (_weight + userWeight >= quorum) {
            canOffboard[term] = true;
        }
        ...
    }

After the LendingTermOffboarding.offboard function is called for such term, the LendingTermOffboarding.cleanup function can be called to change canOffboard[term] for the corresponding term to false.

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/e3d3e581e0e51a9ecf3a5a0c4e4bd4af32552fc0/src/governance/LendingTermOffboarding.sol#L153-L170

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

        ...
    }

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/e3d3e581e0e51a9ecf3a5a0c4e4bd4af32552fc0/src/governance/LendingTermOffboarding.sol#L175-L199

    function cleanup(address term) external whenNotPaused {
        require(canOffboard[term], "LendingTermOffboarding: quorum not met");
        require(
            LendingTerm(term).issuance() == 0,
            "LendingTermOffboarding: not all loans closed"
        );
        require(
            GuildToken(guildToken).isDeprecatedGauge(term),
            "LendingTermOffboarding: re-onboarded"
        );

        ...

        canOffboard[term] = false;
        ...
    }

Before POLL_DURATION_BLOCKS after snapshotBlock is reached for such term, if these LendingTermOffboarding.offboard and LendingTermOffboarding.cleanup functions are called, where snapshotBlock is the same snapshotBlock used in the previous LendingTermOffboarding.supportOffboard function calls for the same term, a user, who has at least 1 vote at the end of snapshotBlock and is not one of the voters who made the previous LendingTermOffboarding.supportOffboard function calls for such term, can call the LendingTermOffboarding.supportOffboard function with the same snapshotBlock for the same term to change such term's canOffboard[term] back to true. This can be done because polls[snapshotBlock][term] remains more than quorum after the LendingTermOffboarding.offboard and LendingTermOffboarding.cleanup functions were called for the corresponding term.

Then, if this offboarded term is re-onboarded in the future, the LendingTermOffboarding.offboard and LendingTermOffboarding.cleanup functions can be directly called without any LendingTermOffboarding.supportOffboard function calls. In this case, even if only less than enough voters support offboarding the corresponding term at that time, such term can still be offboarded unexpectedly.

Proof of Concept

The following steps can occur.

1. An offboarding proposal is created for a term at block.number being 12345678.
2. Multiple voters called the LendingTermOffboarding.supportOffboard function with snapshotBlock being 12345678 for the same term, which changes such term's canOffboard[term] to true after the quorum is reached. This is done before POLL_DURATION_BLOCKS after snapshotBlock is reached.
3. Also before POLL_DURATION_BLOCKS after snapshotBlock, which is 12345678, is reached, the LendingTermOffboarding.offboard and LendingTermOffboarding.cleanup functions are called so the corresponding term's canOffboard[term] is changed to false.
4. Again before POLL_DURATION_BLOCKS after snapshotBlock, which is 12345678, is reached, a user, who has just 1 vote at the end of block 12345678 and is not one of the voters who called the LendingTermOffboarding.supportOffboard function in Step 2, can call the LendingTermOffboarding.supportOffboard function with snapshotBlock being 12345678 for the same term. This changes such term's canOffboard[term] back to true.
5. In the future, such term is re-onboarded but the LendingTermOffboarding.offboard and LendingTermOffboarding.cleanup functions can be called directly without any LendingTermOffboarding.supportOffboard function calls. Although only less than enough voters support offboarding the corresponding term at that time, such term can still be offboarded.

Tools Used

Manual Review

Recommended Mitigation Steps

After a term is offboarded through the LendingTermOffboarding.offboard function call, such term can be locked for a reasonable duration, such as a period of time that equals the duration for POLL_DURATION_BLOCKS. During such lock time period, the LendingTermOffboarding.supportOffboard function would not be allowed to be called for such term.

Assessed type

Context

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #1141

[c4-judge]

Trumpero marked the issue as satisfactory

[Trumpero]

This issue is a dup of #1141 due to the root cause: guild holders are still able to call supportOffboard for an offboarded lending term and set canOffboard to be true before re-onboarding.

[c4-judge]

Trumpero marked the issue as not a duplicate

[c4-judge]

Trumpero marked the issue as duplicate of #1141