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

17 stars 11 forks source link

In the proposeOffboard function of the LendingTermOffboarding contract, there is no verification of whether the passed-in term parameter comes from the same market. A malicious actor may use the proposeOffboard function to offboard lending terms from other different markets. #1165

Closed c4-bot-7 closed 8 months ago

c4-bot-7 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

In the proposeOffboard function of the LendingTermOffboarding contract, there is no verification of whether the passed-in term parameter comes from the same market. A malicious actor may use the proposeOffboard function to offboard lending terms from other different markets.

Proof of Concept

The code for the proposeOffboard function is 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;
    emit OffboardSupport(
        block.timestamp,
        term,
        block.number,
        address(0),
        1
    );
}

It can be seen that the incoming term parameter is not verified to be from the same market.

Different markets have different quorum parameters. Suppose there are two LendingTermOffboarding contracts (contract A, contract B, corresponding to the ETH and Bitcoin markets). The quorum parameter of contract A is lower than that of contract B. Suppose:

  1. The quorum of contract A is 10000, and the quorum of contract B is 50
  2. Because the proposeOffboard function does not verify whether the term parameter comes from the same market, as long as there are 50 votes in contract B, the term in contract A can be offboarded, and the redemption function of the corresponding PSM in contract A will be temporarily suspended.

Tools Used

Manual audit

Recommended Mitigation Steps

Optimize the system logic, in the proposeOffboard function of the LendingTermOffboarding contract, verify whether the incoming term parameter comes from the same market

Assessed type

Governance

0xSorryNotSorry commented 9 months ago

The quorum is the same for different terms

c4-pre-sort commented 9 months ago

0xSorryNotSorry marked the issue as insufficient quality report

c4-judge commented 8 months ago

Trumpero marked the issue as unsatisfactory: Invalid