Ethereum Credit Guild - Byteblockers's results

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/ProfitManager.sol#L342-L405

Vulnerability details

Impact

Liquidators will be able to flash loan mint and stake before liquidating the borrower, extracting maximal potential value. While advantageous for liquidators, this significantly reduces gauge stakers' profits without changing the associated risks.

Proof of Concept

To extract the maximal possible value, bidders (liquidators) will mint with PSM and stake in the gauge they are liquidating. This is because, upon liquidation, onBid calls ProfitManager's notifyPnL, distributing part of the interest to gauge voters. This process is achievable in a single transaction, incentivizing liquidators to do it for every profitable (positive PnL) liquidation.

Example:

1. Borrower misses a payment.
2. Call triggers the auction, reaching a profitable point of 8,000 USDC credit for 8,100 USDC collateral.
3. Alice executes a Flash loan transaction:
   • Flash loan 90,000 USDC
   • Mint 90,000 gUSDC
   • Stake 90,000 gUSDC into the gauge
   • Bid on Bob's loan.
   • Call SGM getRewards
   • Unstake 90,000 gUSDC
   • Redeem 90,360 gUSDC
   • Pay the loan

After Alice bids on Bob's loan, calculations are performed, and ProfitManager's notifyPnL is called with 1,000 USDC to split. PM allocates 400 USDC to the gauge. However, Alice holds 90k out of 100k weight (90%), entitling her to 90% of the gauge's profit (360 USDC).

Alice profits 360 USDC from the FL (460 USDC in total) + the gauge tokens that SGM mints as rewardsRatio (360 with rewardRatio of 1), while the remaining gauge stakers split the remaining 40 USDC. This scenario disincentivizes staking for a given gauge, as liquidation becomes a safer and more profitable alternative.

POC

Gist - https://gist.github.com/0x3b33/cf4349253c7762ab4c3d099ecadbea95 Add to - 2023-12-ethereumcreditguild/test/unit/loan/<name>.sol Run it with - forge test --match-test test_flashLoanExtraProfit

Tools Used

Manual review

Recommended Mitigation Steps

Implementing a dripping mechanism similar to that used with credit tokens (here) may be the most effective solution, albeit making gauges more complex. Alternatively, pausing mint could be considered, but this might only make it more challenging as liquidators can still use flash loans to acquire credits through other means.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L562

Vulnerability details

Impact

Due to the gauge profit distribution mechanism, every borrower would be able to claim back a certain percentage of their repayment. Similarly, individuals with significant capital (whales) can exploit this situation to make substantial profits without assuming any risk.

Proof of Concept

When a borrower makes a payment through either partialRepay or repay, notifyPnL is called. This function sends the profit (interest + start fee) to the Profit Manager, which then distributes the profit to the buffer, credit token holders, gauge voters, and other special addresses. The issue arises from the instant distribution of profits to gauge voters.

if (amountForGuild != 0) {
    uint256 _gaugeWeight = uint256(GuildToken(guild).getGaugeWeight(gauge));
    if (_gaugeWeight != 0) {
        uint256 _gaugeProfitIndex = gaugeProfitIndex[gauge];
        if (_gaugeProfitIndex == 0) {
            _gaugeProfitIndex = 1e18;
        }
        gaugeProfitIndex[gauge] = _gaugeProfitIndex + (amountForGuild * 1e18) / _gaugeWeight;
    }
}

This implies that anyone with capital can claim some of these profits by simply depositing capital before a borrower initiates payment. Afterwards, they can claim the rewards using getRewards and unstake to avoid the risk of slashing.

Example

1. Alice detects that a borrower is going to make a payment of 5k to a gauge with a 90,000 weight.
2. Alice front-runs the borrower, mints, and stakes 10,000 USDC into the gauge.
3. The borrower repays the 5k, and 50% of it goes to the gauge voters (2500 USDC).
4. Alice back-runs the borrower with getRewards where she gets 10% of the rewards (250 USDC + 250 guild), with zero risk.
5. Alice can unstake and redeem while waiting for the next borrower repayment.

This operation can be executed by one whale simultaneously in every market and every term, as the money is only needed to sandwich.

Note that the borrower can perform the same operation, and they even have the ability to flash-loan and stake a huge weight, and thus extract a big amount of their repayment back.

POC

Gist - https://gist.github.com/0x3b33/7ca9b8a4861c96e0b97ad35c4abf5ff9 Add in - security/2023-12-ethereumcreditguild/test/unit/loan/<name>.sol Run it with:

• forge test --match-test test_borrowerFL - to see how a borrower can FL and steal some profits
• forge test --match-test test_sandwichBorrower - to see how a whale can FL and steal some profits

Tools Used

Manual review

Recommended Mitigation Steps

I suggest implementing a mechanism for gauges to drip in a manner similar to how credit does.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L270

Vulnerability details

Impact

The debtCeiling function may return incorrect values. The vulnerability surpasses this requirement, potentially causing borrowers to reduce the debt ceiling below the issuance.

if (issuance != 0) {
    uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
    require(
        issuance <= debtCeilingAfterDecrement,
        "GuildToken: debt ceiling exceeded"
    );
}

Proof of Concept

The debtCeiling final minimum check may be inaccurate, as it doesn't return the actual minimum value. If _hardCap < creditMinterBuffer, it will still return creditMinterBuffer because creditMinterBuffer is compared first to _debtCeiling.

if (creditMinterBuffer < _debtCeiling && creditMinterBuffer < _hardCap) {
    return creditMinterBuffer;
}
if (_hardCap < _debtCeiling) {
    return _hardCap;
}
return _debtCeiling;

Example:

With the current parameters, debtCeiling will return 100,000 instead of 70,000 (which is the hardCap). These parameters are not uncommon, as they are expected for small markets with not much use. A big buffer suggests that loans are rarely taken and a small hardcap indicates volatility.

Below is the math we need to do to reach the final return value. You can follow along with the code.

toleratedGaugeWeight

debtCeilingBefore

remainingDebtCeiling

otherGaugesWeight

maxBorrow

debtCeiling

function debtCeiling(int256 gaugeWeightDelta) public view returns (uint256) {
    ...
    if (creditMinterBuffer < _debtCeiling && creditMinterBuffer < _hardCap) {
        return creditMinterBuffer;
    }
    if (_hardCap < _debtCeiling) {
        return _hardCap;
    }
    return _debtCeiling;
}

Tools Used

Manual review

Recommended Mitigation Steps

Improve the final min check.

function debtCeiling(int256 gaugeWeightDelta) public view returns (uint256) {
    ...
-   if (creditMinterBuffer < _debtCeiling) {
+   if (creditMinterBuffer < _debtCeiling && creditMinterBuffer < _hardCap) {
        return creditMinterBuffer;
    }
    if (_hardCap < _debtCeiling) {
        return _hardCap;
    }
    return _debtCeiling;
}

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/AuctionHouse.sol#L144-L152 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/test/proposals/gips/GIP_0.sol#L175-L179

Vulnerability details

Impact

Whales (accounts with significant cryptocurrency holdings) can exploit the current auction mechanics to guarantee profits through block stuffing.

This is particularly feasible due to the short duration of auctions, allowing these entities to manipulate auction outcomes in their favor.

Proof of Concept

The protocol uses a Dutch auction format with two phases:

• First Phase: Bidders must pay the full debt amount. The collateral percentage starts at 0% and increases with each new block until the auction's midpoint.
• Second Phase: The protocol offers the full collateral and decreases the owed debt by a percentage in each new block, reaching 0% at auction's end. This implies that a bidder could eventually receive the collateral for free.

Bidders are disincentivized to participate in the first phase, as it generally results in a net loss unless there are force majeure market conditions.

Whales can use a block stuffing attack to win large auctions and acquire collateral at significantly reduced prices.

Example scenario:

1. Alice has the following bad debt which is auctioned off:
   • Collateral: 2,000,000 USDC
   • Debt: 1,000 WETH (1 WETH = 2,000 USDC)
2. There are no bidders in the first phase of the auction since this would result in a loss for the bidder.
3. The auction reaches its midpoint. Collateral cost reduces by ~1.14% every block. This is because the second phase is 1150 sec. (as per the GIP_0.sol deployment script), i.e. 88 blocks on Mainnet. The decay rate is thus 100% / 88 ~ 1.14%
4. At the auction's midpoint, Bob executes block stuffing attack. To make sure his attack would succeed, he uses a gas price of 250 Gwei.
5. After 88 blocks, Bob binds in the final block and wins 2,000,000 USDC at 0 ETH cost.

The attack cost is:

Thus, Bob has made a profit of 500,000 USDC.

This strategy, while requiring substantial funds, is a feasible and potentially lucrative attack vector.

The severity is set to Medium given its low likelihood but high impact.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this vulnerability, it is recommended to extend the auction duration. Longer auctions would increase the cost and complexity of block stuffing attacks, reducing the likelihood of such exploits.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L224-L231

Vulnerability details

Impact

If market conditions change, some markets might consider deprecating specific gauges. However, this action could trigger a bank run on the gauge, leading to permanent losses for some lenders, as their credit tokens would be slashed if a borrower leaves with bad debt.

Proof of Concept

When offBoarding a gauge, PSM is paused, preventing the so-called "bank runs." Nevertheless, these bank runs are still likely to occur in the case of the removed gauge, where voters will attempt to exit before any bad debt accrues, otherwise, they face potential slashing. This will be feasible for some but not for all, as voters cannot decrease the gauge weight below its issuance. This limitation is enforced by the GuildToken's _decrementGaugeWeight, which checks if the issuance of the term is below the debtCeiling.

uint256 issuance = LendingTerm(gauge).issuance();
if (issuance != 0) {
    uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
    require(
        issuance <= debtCeilingAfterDecrement,
        "GuildToken: debt ceiling used"
    );
}

If there are still existing borrowers (whose auctions haven't finished), lenders are not allowed to leave. If one of these borrowers causes bad debt, all lenders will be slashed. The pausing mechanism that aims to stop these bank runs is not working correctly, as they are still likely to happen. In this scenario, the first lenders will win (avoiding credit slashing), while the last will lose (being slashed).

Example:

1. GaugeA is deprecated and every borrower is _called.
2. Alice calls decrementGauge in order to leave the gauge.
3. Bob front-runs Alice and leaves before her.
4. When Alice TX executes _decrementGaugeWeight reverts as she is trying to lower the weight bellow the issuance.
5. Eve auction ends and she accrues bad debt for the gauge.
6. Alice gets slashed.

In this example Alice was fast enough to leave, but due to the issuance check she still gets slashed.

Tools Used

Manual review.

Recommended Mitigation Steps

One option is to keep PSM paused and skip this if, if a gauge is removed. This approach will halt the bank run from the gauge and still use the creditMultiplier as a tool for splitting bad debt.

+  if (isGauge(gauge)) { // This will stop deprecated gauges from entering
     uint256 issuance = LendingTerm(gauge).issuance();
     if (issuance != 0) {
          uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
          require(
             issuance <= debtCeilingAfterDecrement,
             "GuildToken: debt ceiling used"
          );
     }
+  }

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L274-L281

Vulnerability details

Impact

Gauges weight will still take effect in markets where there is only one term. This is not desired, as described by the developers, if there is only one gauge, its weight won't matter.This will make voters for this gauge unable to decrease its weight, even though they should be able to do so.

debtCeiling should not go below issuance because if there is just one term, then 100% of the borrows can happen there regardless of the weight (10 or 1e27, it doesn't matter, still 100%, so loans should be unrestricted).

Proof of Concept

The function _decrementGaugeWeight, called by decrementGauge, checks if the new decrease in gauge weight will lower the debt ceiling below the issuance.

uint256 issuance = LendingTerm(gauge).issuance();
if (issuance != 0) {
    uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
    require(issuance <= debtCeilingAfterDecrement, "GuildToken: debt ceiling used");
}

In debtCeiling, we get the gauge weight and add the gaugeWeightDelta (decrease the weight in this case).

uint256 gaugeWeight = GuildToken(_guildToken).getGaugeWeight(address(this));
gaugeWeight = uint256(int256(gaugeWeight) + gaugeWeightDelta);

However, because totalTypeWeight is updated after debtCeiling finishes execution, the math above makes it so gaugeWeight != totalTypeWeight even though there is only one gauge.

This, in turn, will pass by the first else if (which is made for occasions where there is only one gauge) and continue execution below. It will most likely stop at the if below, where it will return debtCeilingBefore, which will cause _decrementGaugeWeight to revert. This is because debtCeilingBefore is already smaller than issuance.

uint256 toleratedGaugeWeight = (gaugeWeight * gaugeWeightTolerance) / 1e18;
uint256 debtCeilingBefore = (totalBorrowedCredit * toleratedGaugeWeight) / totalWeight;
if (_issuance >= debtCeilingBefore) {
    return debtCeilingBefore;
}

The above is likely, as we have mentioned that gaugeWeight != totalTypeWeight, which will make this calculation push totalBorrowedCredit downwards (i.e., it will make it smaller since totalWeight > toleratedGaugeWeight).

uint256 debtCeilingBefore = (totalBorrowedCredit * toleratedGaugeWeight) / totalWeight;

Example:

With the example above, our current user is trying to unstake 5,000 weight from the gauge. As it's the only gauge in the market, debtCeiling should return hardCap or creditMinterBuffer (whichever is the smaller one). Let's check the math and find out.

1. gaugeWeight will be calculated as 15,000.

$\text{gaugeWeight} = \text{gaugeWeight} - \text{gaugeWeightDelta} = 20000e18 - 5000e18 = 15000e18$

2. totalWeight is 20,000 as it's reduced in decrementGauge after _decrementGaugeWeight finishes.

3. This will skip else if since gaugeWeight != totalWeight.

4. toleratedGaugeWeight will be 18,000.

$\text{toleratedGaugeWeight} = \frac{\text{gaugeWeight} \times \text{gaugeWeightTolerance}}{1e18} = \frac{15000e18 \times 1.2e18}{1e18} = 18000e18$

5. This will make debtCeilingBefore be 18,900.

$\text{debtCeilingBefore} = \frac{\text{totalBorrowedCredit} \times \text{toleratedGaugeWeight}}{\text{totalWeight}} = \frac{21000e18 \times 18000e18}{20000e18} = 18900e18$

6. We will enter the if below where it will return debtCeilingBefore.

if (_issuance >= debtCeilingBefore) {
    return debtCeilingBefore;
}

7. _decrementGaugeWeight will revert as debtCeilingBefore < issuance.

uint256 issuance = LendingTerm(gauge).issuance();
if (issuance != 0) {
    uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
    require(issuance <= debtCeilingAfterDecrement, "GuildToken: debt ceiling used");
}

Note that, of course, the user can make 5 separate TX and withdraw those 5k, 1k at a time (this is possible as gaugeWeight * 1.2 > totalWeight and entering this if). However, it will be unnecessary, and the solution I have recommended will fix the issue without asking the user to redo his TX.

Tools Used

Manual review.

Recommended Mitigation Steps

Add a check here, before updating the gauge weight, to see if there is only one gauge.

        uint256 gaugeWeight = GuildToken(_guildToken).getGaugeWeight(address(this));
+      uint256 totalWeight = GuildToken(_guildToken).totalTypeWeight(gaugeType);
+      if (gaugeWeight == totalWeight) {
+           return _hardCap < creditMinterBuffer ? _hardCap : creditMinterBuffer;
+      }
        gaugeWeight = uint256(int256(gaugeWeight) + gaugeWeightDelta);
        uint256 gaugeType = GuildToken(_guildToken).gaugeType(address(this));
-       uint256 totalWeight = GuildToken(_guildToken).totalTypeWeight(gaugeType);

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L394-L397

Vulnerability details

Impact

There is an inconsistency in the calculation of the debtCeiling between the borrow() function and the debtCeiling() function. This not only leads to operational discrepancies but also impacts liquidity utilization. Specifically, the more restrictive debtCeiling in the borrow() function results in underutilized liquidity, which in turn could lead to missed profit opportunities for lenders.

Proof of Concept

There is an inconsistency in the way borrow() calculates a much smaller value for debtCeiling than the actual debtCeiling() function. This renders this check useless since borrow prevents issuance from going even close to the actual debt ceiling.

uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
require(issuance <= debtCeilingAfterDecrement, "GuildToken: debt ceiling used");

Let's take the parameters below to illustrate the issue and follow along the two calculations:

1. borrow() function's calculation method:

• This function calculates the debtCeiling using a simpler formula:

• Applying the provided parameters (Gauge Weight: 50,000, Total Borrowed Credit: 70,000, Total Weight: 100,000, and Weight Tolerance: 1.2), the resulting debtCeiling is 42,000.

2. debtCeiling() function's calculation method:

• The formula used here is more complex so we will it break it down below:

• This method involves several intermediate steps to arrive at the debtCeiling. The process includes:
  • Calculating the tolerated gauge weight.
  • Determining the initial debt ceiling before any new borrowings.
  • Computing the remaining debt ceiling after factoring in current issuance.
  • Establishing the weight of other gauges.
  • Determining the maximum borrowable amount.
  • Adding the current issuance to the maximum borrow to get the final debtCeiling.

toleratedGaugeWeight

debtCeilingBefore

remainingDebtCeiling

otherGaugesWeight

maxBorrow

debtCeiling

With the same parameters, this method results in a debtCeiling of 75,000.

The lower debtCeiling set by the borrow() function (42,000) significantly restricts the amount that can be borrowed compared to what is actually permissible as per the debtCeiling() function (75,000).

This discrepancy leads to a situation where a portion of the liquidity remains unused. In a lending scenario, unused liquidity equates to lost income opportunities for lenders, as these funds are not being loaned out and thus not generating interest.

Tools Used

Manual review.

Recommended Mitigation Steps

Unify the debtCeiling calculation method which is used across the protocol.

Assessed type

Error