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

9 stars 5 forks source link

DoS in `LendingTermOnboarding` via proposal creation and cancellation #1243

Closed c4-bot-10 closed 5 months ago

c4-bot-10 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

The LendingTermOnboarding contract is vulnerable to a Denial of Service (DoS) attack. This vulnerability arises from the ability for an attacker to repeatedly create and cancel proposals every time a term is created and after the MIN_DELAY_BETWEEN_PROPOSALS period.

This vulnerability is related to https://github.com/OpenZeppelin/openzeppelin-contracts/security/advisories/GHSA-5h3x-9wvq-w4m2

The vulnerability referenced above was fixed in v4.9.1 of OZ's contracts by introducing opt-in frontrunning protection, but that protection isn't available in LendingTermOnboarding since the contract disallows custom proposals and enforces a deterministic description in proposeOnboard() based on the term address.

An attacker can exploit this vulnerability by observing when someone creates a term, then calling proposeOnboard() themselves. They can then cancel the proposal using the cancel() method provided by the Governor contract, from which LendingTermOnboarding inherits. By repeating this action every MIN_DELAY_BETWEEN_PROPOSALS, the attacker can effectively prevent the proposing of new terms.

function proposeOnboard(
    address term
) external whenNotPaused returns (uint256 proposalId) {
    // Check that the term has been created by this factory
    require(created[term] != 0, "LendingTermOnboarding: invalid term");
    // Check that the term was not subject to an onboard vote recently
    require(
        lastProposal[term] + MIN_DELAY_BETWEEN_PROPOSALS < block.timestamp,
        "LendingTermOnboarding: recently proposed"
    );
    lastProposal[term] = block.timestamp;
    // ...
}

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/LendingTermOnboarding.sol#L181 https://github.com/OpenZeppelin/openzeppelin-contracts/blob/fd81a96f01cc42ef1c9a5399364968d0e07e9e90/contracts/governance/Governor.sol#L348

Proof of Concept

Consider the following scenario:

  1. Alice creates a term using the createTerm() function.
  2. Bob observes Alice's transaction and calls proposeOnboard() with the term Alice just created before she can do so herself
  3. Bob then cancels the proposal using the cancel() method.
  4. Bob repeats this process every MIN_DELAY_BETWEEN_PROPOSALS, effectively preventing Alice and other legitimate users from proposing new terms.

Tools Used

Manual review

Recommended Mitigation Steps

Allow users to provide a custom description string to attach to the one generated in getOnboardProposeArgs(). This way, they would also be able to benefit from the frontrunning protection implemented in OZ's Governor.

Assessed type

Governance

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as duplicate of #1125

c4-judge commented 5 months ago

Trumpero changed the severity to QA (Quality Assurance)

c4-judge commented 4 months ago

Trumpero marked the issue as grade-c