Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can manipulate the debt ceiling to borrow beyond it #307

Closed
c4-bot-10 opened this issue Dec 21, 2023 · 6 comments
Closed

Can manipulate the debt ceiling to borrow beyond it #307

c4-bot-10 opened this issue Dec 21, 2023 · 6 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-10
Copy link
Contributor

c4-bot-10 commented Dec 21, 2023

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L383

Vulnerability details

Impact

The debt ceiling can be manipulated by temporarily increasing totalBorrowedCredit, allowing for attackers to borrow well beyond the intended debt ceiling, DoS'ing gauge voters as a result.

Proof of Concept

In LendingTerm._borrow, we compute the debt ceiling by calculating the gauge allocation for the term with a quantity of totalBorrowedCredit + borrowAmount.

uint256 _debtCeiling = (GuildToken(refs.guildToken)
    .calculateGaugeAllocation(
        address(this),
        totalBorrowedCredit + borrowAmount
    ) * gaugeWeightTolerance) / 1e18;

We can see below in calculateGaugeAllocation that since the quantity param is the numerator, as it increases, the return value increases.

function calculateGaugeAllocation(
    address gauge,
    uint256 quantity
) external view returns (uint256) {
    if (_deprecatedGauges.contains(gauge)) return 0;

    uint256 total = totalTypeWeight[gaugeType[gauge]];
    if (total == 0) return 0;
    uint256 weight = getGaugeWeight[gauge];

    return (quantity * weight) / total;
}

We can see in the ProfitManager that totalBorrowedCredit is computed as follows.

function totalBorrowedCredit() external view returns (uint256) {
    return
        CreditToken(credit).targetTotalSupply() -
        SimplePSM(psm).redeemableCredit();
}

So if we want to increase the debt ceiling, we can increase the Credit token supply without increasing the SimplePSM balance.

If we have a term (term B) which has a lot of room until its debt ceiling and a term (term A) which we want to borrow beyond the debt ceiling, we can do so as follows:

  1. Borrow from term B up to the debt ceiling (this increases totalBorrowedCredit which increases the debt ceiling of all other terms)
  2. Borrow from term A up to the new debt ceiling
  3. Repay loan from term B (causes debt ceiling to go down, term A's debt ceiling is now less than its issuance)
  • Note that we have to wait until the next block before we can repay this loan

Since we don't need to redeem our credit from term B in the SimplePSM, the totalBorrowedCredit increases which gives us a higher debt ceiling for the term we actually want to borrow from.

The result of this attack is the attacker getting much more liquidity than intended, increasing the risk of the term beyond its intended limit. This also results in the gauge voters for term A having their gauge weight stuck since they can't decrement their gauge weight if it results in the debt ceiling being less than the issuance.

Tools Used

  • Manual review

Recommended Mitigation Steps

It's unsafe to use totalBorrowedCredit to determine a safe debt ceiling as it can always be manipulated in some way. Instead, I would recommend using a Governor controlled fixed total debt ceiling in which the individual term debt ceilings are just computed based on their share of the total gauge weight.

Assessed type

Math

@c4-bot-10 c4-bot-10 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 21, 2023
c4-bot-10 added a commit that referenced this issue Dec 21, 2023
@0xSorryNotSorry
Copy link

It looks like this is the intended behaviour as the total debt ceiling is the same during these actions.

@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Jan 5, 2024
@Trumpero
Copy link

Trumpero commented Jan 22, 2024

The result of this attack is the attacker getting much more liquidity than intended, increasing the risk of the term beyond its intended limit. This also results in the gauge voters for term A having their gauge weight stuck since they can't decrement their gauge weight if it results in the debt ceiling being less than the issuance.

Invalid statement. It's intended design and debt ceiling can be less than issuance by multiple reasons.

@c4-judge
Copy link
Contributor

Trumpero marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jan 22, 2024
@kadenzipfel
Copy link

This is falsely marked as invalid.

debt ceiling can be less than issuance by multiple reasons.

While it's true we can reasonably end up in this state, this should only be causable by gauge voters, who are essentially lenders. It's reasonable for a lender to limit the terms of a loan as they're the party that's affected if the loan goes under.

As described, the borrower can continually increase the debt ceiling by increasing the totalBorrowedCredit, allowing them to borrow effectively as much as they would like. This is especially problematic considering the whole purpose of the debtCeiling is to limit the amount that users can borrow. This can of course also be used as a griefing attack against gauge voters, causing them to be unable to exit their positions.

@Trumpero
Copy link

Trumpero commented Feb 8, 2024

@kadenzipfel I still don't see any issue here. Issuance is limited by the hardcap, and the debtCeiling is used to determine the relative issuance of lending terms. In this case, the debtCeiling of term A exceeds the issuance, but issuance is still below the hardcap. After that, only borrows in term B are allowed, so it's not different when the issuance of term A reaches the debtCeiling. It's the intended design of the debtCeiling to rebalance issuance of lending terms based on their gauge weights.
Additionally, it's expected behavior that gauge voters can't decrement gauge weight if their decrements decrease the debtCeiling below the issuance, even when the debtCeiling is already smaller. They can still exit their Guild tokens by off-boarding the lending term then calling the loans without occurring bad debt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

7 participants