code-423n4 / 2023-12-ethereumcreditguild-findings

17 stars 11 forks source link

LendingTermOffboarding : once a term is proposed to Offboard and missed from off boarding, it never be proposed for offboard again. #1145

Closed c4-bot-1 closed 8 months ago

c4-bot-1 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

A lending term can not be off boarded again if is initiated to off board and missed to do so.

Proof of Concept

ECG has contract LendingTermOffboarding.sol which has logic implemented to off board a lending term. From contract page : ///This contract works somewhat similarly to a Veto governor: any GUILD holder can poll for the removal /// of a lending term, and if enough GUILD holders vote for a removal poll, the term can be offboarded without delay. /// When a term is offboarded, no new loans can be issued, and GUILD holders cannot vote for the term anymore. /// After a term is offboarded, all the loans have to be called, then the term can be cleaned up (roles).

Following function would be called by anyone to start off board. proposeOffboard.

    function proposeOffboard(address term) external whenNotPaused {
        require(
            polls[block.number][term] == 0, ---------------->>> it must be to propose
            "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 ------->>> once started, this is set to 1
        lastPollBlock[term] = block.number;
        emit OffboardSupport(
            block.timestamp,
            term,
            block.number,
            address(0),
            1
        );
    }

Once sufficient votes are met, the lending term would flagged for offboard by calling the function supportOffboard . https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/LendingTermOffboarding.sol#L116-L124

    function supportOffboard(
        uint256 snapshotBlock,
        address term
    ) external whenNotPaused {
        require(
            block.number <= snapshotBlock + POLL_DURATION_BLOCKS, ---->> pls not this. this is there to ensure that the off boarding is not too old
            "LendingTermOffboarding: poll expired"
        );
        uint256 _weight = polls[snapshotBlock][term];
....
..
.

The issue here is, the check that is referred in above code snippet. In a case, if the user calls the function proposeOffboard and propose for off board. But the time has passed snapshotBlock + POLL_DURATION_BLOCKS, the lending term can not be offboarded.

later, if the same lending term wanted to be off boarded due to market condition, it can not be done due to the check at line_91 from proposing again. Also, supportOffboard will revert at line_121.

If somebody wants to prevent off boarding any of the lending term, they will call the offboard function as soon as the lending term is created. Once the time is passed, the lending term can not be proceeded for offboarding.

Tools Used

Manual review.

Recommended Mitigation Steps

We would suggest to added new function which would be called by owner/admin to reset the off board settings once the snapshotBlock + POLL_DURATION_BLOCKS duration is passed.

Assessed type

Other

0xSorryNotSorry commented 8 months ago

block.number <= snapshotBlock + POLL_DURATION_BLOCKS, ---->> pls not this. this is there to ensure that the off boarding is not too old Above is for voting, not for offboarding functionality.

If the term reached to the Quorum for offboarding, it can be done anytime via offboard(). Else, for the new poll, the proposer can re-call the proposeOffboard once the poll is over.

c4-pre-sort commented 8 months ago

0xSorryNotSorry marked the issue as insufficient quality report

c4-judge commented 8 months ago

Trumpero marked the issue as unsatisfactory: Invalid