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

Auction manipulation by block stuffing and reverting on ERC-777 hooks #685

Open
c4-bot-5 opened this issue Dec 27, 2023 · 14 comments
Open
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 disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) M-16 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sufficient quality report This report is of sufficient quality

Comments

@c4-bot-5
Copy link
Contributor

Lines of code

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

Vulnerability details

HIGH: Auction manipulation by block stuffing and reverting on ERC-777 hooks

GitHub Links

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

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

Summary

The protocol stated out in the C4 description that the deployment script of the protocol, located in test/proposals/gips/GIP_0.sol is also in scope, as protocol deployment/configuration mistakes could be made. A low immutable auction duration set in this deployment script can lead to profitable block stuffing attacks on the desired L2 chains. This attack vector can be further improved under the condition that the collateral token is ERC-777 compatible.

Vulnerability Details

The auction house contract is deployed with the following parameters (auctionDuration and midPoint are immutables):

AuctionHouse auctionHouse = new AuctionHouse(
    AddressLib.get("CORE"),
    650, // midPoint = 10m50s
    1800 // auctionDuration = 30m
);

During the first half of the auction (before midPoint), an increasing amount of the collateral is offered, for the full CREDIT amount.

During the second half of the action (after midPoint), all collateral is offered, for a decreasing CREDIT amount.

The calculation can be seen in the getBidDetail function:

uint256 public immutable midPoint;
uint256 public immutable auctionDuration;

function getBidDetail(bytes32 loanId) public view returns (uint256 collateralReceived, uint256 creditAsked) {
  // check the auction for this loan exists
  uint256 _startTime = auctions[loanId].startTime;
  require(_startTime != 0, 'AuctionHouse: invalid auction');

  // check the auction for this loan isn't ended
  require(auctions[loanId].endTime == 0, 'AuctionHouse: auction ended');

  // assertion should never fail because when an auction is created,
  // block.timestamp is recorded as the auction start time, and we check in previous
  // lines that start time != 0, so the auction has started.
  assert(block.timestamp >= _startTime);

  // first phase of the auction, where more and more collateral is offered
  if (block.timestamp < _startTime + midPoint) {
    // ask for the full debt
    creditAsked = auctions[loanId].callDebt;

    // compute amount of collateral received
    uint256 elapsed = block.timestamp - _startTime; // [0, midPoint[
    uint256 _collateralAmount = auctions[loanId].collateralAmount; // SLOAD
    collateralReceived = (_collateralAmount * elapsed) / midPoint;
  }
  // second phase of the auction, where less and less CREDIT is asked
  else if (block.timestamp < _startTime + auctionDuration) {
    // receive the full collateral
    collateralReceived = auctions[loanId].collateralAmount;

    // compute amount of CREDIT to ask
    uint256 PHASE_2_DURATION = auctionDuration - midPoint;
    uint256 elapsed = block.timestamp - _startTime - midPoint; // [0, PHASE_2_DURATION[
    uint256 _callDebt = auctions[loanId].callDebt; // SLOAD
    creditAsked = _callDebt - (_callDebt * elapsed) / PHASE_2_DURATION;
  }
  // second phase fully elapsed, anyone can receive the full collateral and give 0 CREDIT
  // in practice, somebody should have taken the arb before we reach this condition.
  else {
    // receive the full collateral
    collateralReceived = auctions[loanId].collateralAmount;
    //creditAsked = 0; // implicit
  }
}

This means that as longer the auction goes till a bid is made (which instantly buys the auction), the more profit can be made by executing the auction.

The following conditions allow an attacker to manipulate auctions by stuffing blocks to increase profits:

  • auctionDuration is set to 1800 seconds (30min) in the deployment script
  • auction midPoint is set to 650 seconds (10m50s) in the deployment script
  • the protocol will be deployed on L2s let's take optimism for example (as this chain was mentioned by the devs in discord):
    • blocks in optimism are minted every 2s
    • the block gas limit of optimism is 30M as in ethereum mainnet
    • at the time of writting this the cost of stuffing a full block on optimism is around $6.39 (https://www.cryptoneur.xyz/en/gas-fees-calculator?gas-input=15000000&gas-price-option=on)
    • therefore prevent a bid for one minute costs $6.39 * 30 blocks = $191.7
    • therefore prevent a bid for 30 minutes costs $191.7 * 30 minutes = $5751
    • but the attacker will almost never need to stuff blocks for around 30 minutes as the system of the auction is not profitable in the beginning and it starts to be really profitable after the midpoint, and it also starts to be much more damaging to the system at this point. Which is after 10m50s and the costs to stuff blocks for 10m50s is $191.7 * 10.5 minutes = $2012.85 and as mentioned before the auction is not profitable at the beginning, therefore no one will bid on second one and the attacker does not need to stuff blocks instantly. This means the attack will most likely cost less than $2000.
  • therefore if at any given timestamp the profit of the auction outweighs the cost of stuffing blocks for the time till the deal becomes profitable for the attacker a system damaging incentive appears and manipulating auctions becomes a profitable attack vector.

But the impact increases further in terms of griefing as loss for terms can occur after the midPoint which will instantly lead to slashing and therefore all stakers of the given term will lose all their credit tokens weighted on this term.

The following code snippets showcase the slashing mechanism that lead to a total loss for the stakers if the term receives any loss during these block stuffing attack:

function bid(bytes32 loanId) external {
    ...

    LendingTerm(_lendingTerm).onBid(
        loanId,
        msg.sender,
        auctions[loanId].collateralAmount - collateralReceived, // collateralToBorrower
        collateralReceived, // collateralToBidder
        creditAsked // creditFromBidder
    );

    ...
}

function onBid(
    bytes32 loanId,
    address bidder,
    uint256 collateralToBorrower,
    uint256 collateralToBidder,
    uint256 creditFromBidder
) external {
    ...

    int256 pnl;
    uint256 interest;
    if (creditFromBidder >= principal) {
        interest = creditFromBidder - principal;
        pnl = int256(interest);
    } else {
        pnl = int256(creditFromBidder) - int256(principal);
        principal = creditFromBidder;
        require(
            collateralToBorrower == 0,
            "LendingTerm: invalid collateral movement"
        );
    }

    ...

    // handle profit & losses
    if (pnl != 0) {
        // forward profit, if any
        if (interest != 0) {
            CreditToken(refs.creditToken).transfer(
                refs.profitManager,
                interest
            );
        }
        ProfitManager(refs.profitManager).notifyPnL(address(this), pnl);
    }

    ...
}

function notifyPnL(
    address gauge,
    int256 amount
) external onlyCoreRole(CoreRoles.GAUGE_PNL_NOTIFIER) {
    ...

    // handling loss
    if (amount < 0) {
        uint256 loss = uint256(-amount);

        // save gauge loss
        GuildToken(guild).notifyGaugeLoss(gauge);

        // deplete the term surplus buffer, if any, and
        // donate its content to the general surplus buffer
        if (_termSurplusBuffer != 0) {
            termSurplusBuffer[gauge] = 0;
            emit TermSurplusBufferUpdate(block.timestamp, gauge, 0);
            _surplusBuffer += _termSurplusBuffer;
        }

        if (loss < _surplusBuffer) {
            // deplete the surplus buffer
            surplusBuffer = _surplusBuffer - loss;
            emit SurplusBufferUpdate(
                block.timestamp,
                _surplusBuffer - loss
            );
            CreditToken(_credit).burn(loss);
        }
    } ...
}

function notifyGaugeLoss(address gauge) external {
    require(msg.sender == profitManager, "UNAUTHORIZED");

    // save gauge loss
    lastGaugeLoss[gauge] = block.timestamp;
    emit GaugeLoss(gauge, block.timestamp);
}

/// @notice apply a loss that occurred in a given gauge
/// anyone can apply the loss on behalf of anyone else
function applyGaugeLoss(address gauge, address who) external {
    // check preconditions
    uint256 _lastGaugeLoss = lastGaugeLoss[gauge];
    uint256 _lastGaugeLossApplied = lastGaugeLossApplied[gauge][who];
    require(
        _lastGaugeLoss != 0 && _lastGaugeLossApplied < _lastGaugeLoss,
        "GuildToken: no loss to apply"
    );

    // read user weight allocated to the lossy gauge
    uint256 _userGaugeWeight = getUserGaugeWeight[who][gauge];

    // remove gauge weight allocation
    lastGaugeLossApplied[gauge][who] = block.timestamp;
    _decrementGaugeWeight(who, gauge, _userGaugeWeight);
    if (!_deprecatedGauges.contains(gauge)) {
        totalTypeWeight[gaugeType[gauge]] -= _userGaugeWeight;
        totalWeight -= _userGaugeWeight;
    }

    // apply loss
    _burn(who, uint256(_userGaugeWeight));
    emit GaugeLossApply(
        gauge,
        who,
        uint256(_userGaugeWeight),
        block.timestamp
    );
}

This attack vector can be further improved under the condition that the collateral token is ERC-777 compatible. It is advised to first read the report called Bad debt can occur if the collateral token blacklists a borrower leading to total loss of stake for all lenders on that term which showcases how the auction time is increased till the midPoint of the auction if transferring the collateral tokens to the borrower reverts.

The attack path would be as follows:

  • attacker borrows a loan (with a ERC-777 compatible collateral token) using a contract that revert on receiving the collateral back if the tx.origin != address(attacker)
  • the attacker receives the credit token from the loan and deposits collateral into the protocol
  • the attacker does not repay the loan after the first partial duration and call it for auction
  • as showcased in the mentioned report every call till the midPoint will revert as trying to transfer the collateral back to the attacker will revert (this will drastically decrease the costs for the attacker as he does not need to stuff blocks till the midPoint is reached)
  • after the midPoint is reached the attacker can start stuffing blocks till bad debt occurs, at this time the attacker can bid on the auction and will therefore buy the full collateral back for less credit tokens than the attacker received for the loan
  • if it is possible for the attacker to stuff blocks and wait till the asked credit amount goes to 0 the attacker was able to steal almost the full loan amount (which can potentially be way bigger than the gas amount for stuffing the blocks as the calculation in the beginning showed)

Impact

The attacker can prevent other users from bidding on the auction and therefore manipulate the auction to a point where the attacker would be able to buy the full collateral for almost zero credit tokens. As loss for the term occurs in such an event, all stakers of the given term will lose all their credit tokens weighted on this term. If the given collateral token is ERC-777 compatible, the costs of such an attack can be drastically reduced. And the attack can potentially become a self liquidation attack.

Recommendations

Increase the auction duration, as the longer the auction goes the less profitable such an attack would be and implement the mentioned fix in the Bad debt can occur if the collateral token blacklists a borrower leading to total loss of stake for all lenders on that term report.

Assessed type

MEV

@c4-bot-5 c4-bot-5 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 27, 2023
c4-bot-2 added a commit that referenced this issue Dec 27, 2023
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Jan 2, 2024
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as primary issue

@eswak
Copy link

eswak commented Jan 12, 2024

I'm skeptical of this being a valid finding (despite fomo3d), at least it's not a high. It is true that there is more spam/censorship risk on Optimism & Arbitrum. On mainnet liquidators can use flashbots etc, while on Arbitrum the sequencer is first come first served. If the liquidator raises the gas price of one tx, the spammer has to raise the gas price of all tx that fill the block. Costs can get out of hand pretty quickly. Take-away I think is that on L2s, the auctions should be long enough to account for this. We've also been discussing very long auctions internally to minimize liquidity risk (and allow people to have time to bridge assets in order to participate in auctions), which also mitigates this vector.

@c4-sponsor
Copy link

eswak (sponsor) acknowledged

@c4-sponsor c4-sponsor added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Jan 12, 2024
@c4-sponsor
Copy link

eswak marked the issue as disagree with severity

@c4-sponsor c4-sponsor added the disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) label Jan 12, 2024
@c4-judge
Copy link
Contributor

Trumpero changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax grade-b and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jan 26, 2024
@c4-judge
Copy link
Contributor

Trumpero marked the issue as grade-b

@c4-judge
Copy link
Contributor

This previously downgraded issue has been upgraded by Trumpero

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value and removed downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Jan 31, 2024
@Trumpero
Copy link

After reviewing again, I consider this to be an issue of block stuffing attack on the auction—a hypothetical attack path that could be profitable for attacker and result in losses for protocol. So I believe this should be a med, and only #463 is duplicate.

@c4-judge
Copy link
Contributor

Trumpero marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 31, 2024
@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Jan 31, 2024
@c4-judge
Copy link
Contributor

Trumpero marked the issue as selected for report

@0xbtk
Copy link

0xbtk commented Feb 2, 2024

Hey @Trumpero, could you please take another look?

The PoC seems impractical, assuming a constant gas price, which doesn't reflect reality. As the sponsor noted, "Costs can get out of hand pretty quickly," and auction profits wouldn't cover such expenses.

Also, note that ERC777 is excluded from the scope, as mentioned in the Discord channel by the sponsor. Check @RunSoul22's comment for details.

Edit: The report didn't consider EIP1559:

With EIP-1559, the base fee will increase and decrease by up to 12.5% after blocks are more than 50% full. For example, if a block is 100% full the base fee increases by 12.5%; if it is 50% full the base fee will be the same; if it is 0% full the base fee would decrease by 12.5%.

Ref: https://consensys.io/blog/what-is-eip-1559-how-will-it-change-ethereum

@cosine-function
Copy link

@0xbtk The use of ERC-777 tokens is optional for this attack vector and only showcases a possibility to reduce the costs of the attack. Also, the maximum increase of the base fee on optimism (which was the blockchain taken as an example in this report) is 10% (https://docs.optimism.io/chain/differences#eip-1559). You are right that the costs of the attack are increased, but could still be profitable depending on the size of the loan. The attacker does also not need to stuff blocks for the full 30 minutes. Even one or a few blocks could be profitable depending on the size of the loan and the costs of stuffing a few blocks when the first one costs $6 and increases by 10% is pretty low.

@Trumpero
Copy link

Trumpero commented Feb 8, 2024

Although it is very unlikely to make profits, I still consider it as a possible hypothetical path with low likelihood. With the current configuration (30 minutes for auction duration), it doesn't guarantee that block stuffing might not be possible, so this issue should be a med.

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 disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) M-16 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

10 participants