Disclosed Griefing Bug Analysis

Simple Summary

OpenZeppelin had conducted an analysis of the vulnerability affecting the Comet Base WETH market that was disclosed by @brrito and has since been fixed with Proposal 195. We first conducted this analysis privately for the Pause Guardian to discuss potential mitigation strategies and now share the results here to provide the community additional information on the vulnerability, its feasibility and our recommendations to mitigate the issue going forward.

What was disclosed?

The disclosure shows it is possible to withdraw and transfer small amounts out of the Base WETH Comet market without a collateralized position. I.e. The exploiter needs no collateral and no previous WETH balances to extract a small amount (<0.00000001 WETH) from the contract.

What are the details of the issue?

A user without a balance can transfer or withdraw small amounts of base asset tokens without collateral as a result of a rounding issue in the liquidity calculation. The transfer or withdrawal creates a small, negative principal balance. The isBorrowCollateralized function calculates a user’s liquidity. If liquidity is zero or greater, the position is considered collateralized and the transfer succeeds.

Let’s consider the calculation for liquidity in isBorrowCollateralized

int liquidity = signedMulPrice(
    presentValue(principal),
    getPrice(baseTokenPriceFeed),
    uint64(baseScale)
);

So liquidity is returned from signedMulPrice. This function is two operations, a multiplication and a division.

function signedMulPrice(int n, uint price, uint64 fromScale) internal pure returns (int) {
    return n * signed256(price) / int256(uint256(fromScale));
}

:arrow_forward: If the denominator is ever larger than the numerator, the return value will be zero.

The Denominator

The denominator, uint64(baseScale) is equal to ten raised to the power of the number of decimals the base asset has. Specifically,

/// @notice The scale for base token (must be less than 18 decimals)
/// @dev uint64
uint public override immutable baseScale;
//  ...
baseScale = uint64(10 ** decimals_);

Note that decimals_ here is constrained to match the ERC20 decimals() of the base asset contract and must also be less than the MAX_BASE_DECIMALS which is hard-coded in Comet markets to be eighteen. So the explicit casting is non-problematic. And the denominator of the fraction can take on values between 0 and 1e18 but is fixed for a market by the decimals of the asset. In the case of WETH, which has 18 decimals, baseScale = 1e18. For USDC, this is 1e6.

The Numerator

Let’s just assume, for the moment, that the two values returned from presentValue and getPrice that are multiplied together have no rounding issues themselves. What values can they take?

presentValue takes a user’s principal amount, a uint104 value (max ~2e31), and multiplies it by the borrow or supply index. Realistically, the indexes are about equal to 1e15 and the entire totalSupplyBase/totalBorrowBase amounts (which are the principal amounts for the markets) when multiplied through this function (see totalSupply() and totalBorrow()) get maximum values ~ 1e14 for USDC markets and ~4e22 for WETH markets.

The getPrice function returns a uint256, but the decimals for each comet market price feed is hard-coded to 8. Each market price, under good conditions, returns values close to 1e8.

Remember, though, that all these max values are to give a sense for the scale we’re talking about. But the issue only arises if the user’s position is small enough to multiply by the price to be less than the baseScale value in the denominator. Given a price feed under usual conditions we arrive at this formula:

:arrow_forward: Liquidity will round down to zero if

presentValue < baseScale / price

For Asset Decimals (D) and Price significant figures (P):
presentValue < 1e(D) / 1e(P) 

presentValue < 1e(D-P)

In simple terms, if a user position is smaller than 10 to the power of the asset decimals minus the price feed’s current returned significant figures, their liquidity will round to zero here

Caveats, Implications

Normally, price feeds are rather stable at 1e8 or one as an 8 decimal value. In fact, for the WETH market the base asset price is fixed to this value. This means the above formula can be simplified under normal market conditions. The rounding issue will occur, under normal conditions, if a user’s base position becomes smaller than one to the power of the base asset decimals minus eight. And given these stable conditions, the rounding will not occur in markets where the base decimals are less than or equal to eight (e.g. USDC or WBTC).

Of course, this thinking depends on the price feeds being relatively stable. Should a price feed drop by 90% (so, 10% of what it is usually), then the position that can round to zero can be multiplied by ten (or the asset decimals minus seven). Continuing this scenario, consider a price feed failure that reports 1 wei as a value. The presentValue that rounds to zero approaches 1eD (as P is essentially zero). Therefore, 1 unit of base token (e.g. 1 USDC or 1WETH) becomes the maximum amount you could withdraw from the protocol in this scenario. This could be profitable at current gas prices in that scenario. However, in such a catastrophic failure of the oracle, large amounts of the base asset could be withdrawn with very little collateral so there would be larger attacks to worry about.

It’s worth mentioning that even though the rounding error affects the liquidity calculation, the attack will still store the principal and reward tracking indexes under the UserBasic data in the updateBasePrincipal function. For the current market, the borrow index speed is zero, and no rewards will be given to the user. However, in any other market that has a non zero value, the malicious user might collect rewards due to the borrow state after time goes by.

Due Diligence On The Numerator Functions Rounding

Here are the functions called from inside the signedMulPrice function that we assumed had no rounding issues above.

function presentValue(int104 principalValue_) internal view returns (int256) {
    if (principalValue_ >= 0) {
        return signed256(presentValueSupply(baseSupplyIndex, uint104(principalValue_)));
    } else {
        return -signed256(presentValueBorrow(baseBorrowIndex, uint104(-principalValue_)));
    }
}

function presentValueBorrow(uint64 baseBorrowIndex_, uint104 principalValue_) internal pure returns (uint256) {
    return uint256(principalValue_) * baseBorrowIndex_ / BASE_INDEX_SCALE;
}

function getPrice(address priceFeed) override public view returns (uint256) {
    (, int price, , , ) = IPriceFeed(priceFeed).latestRoundData();
    if (price <= 0) revert BadPrice();
    return uint256(price);
}

The following points regarding presentValueBorrow apply without loss of generality to presentValueSupply as well.

Regarding presentValueBorrow, there will be a rounding issue if the denominator of BASE_INDEX_DECIMALS is larger than the numerator (and the numerator is not zero). This will never occur because baseBorrowIndex is set to BASE_INDEX_DECIMALS on construction. Therefore the result of the multiplication will always be larger or zero. The casting to int256 is a safe casting function so no casting issue exists there.

Regarding getPrice, this function rejects prices below zero, so it will only ever take values between zero and type(int256).max (~5.8e76) which is below the type(uint256).max (~1.2e77). So that unsafe, explicit cast is non-problematic.

What are the economics of the exploit?

The Base network hosts the market that could be exploited. Such a network possesses an execution cost significantly lower than mainnet and due to the low congestion of the network, its gas price can be as low as 500 wei (seen in recent transactions).

Added to the execution cost, a transaction must pay the fees for publishing the transaction in L1, which is subject to the changes of the mainnet gas price. Usually, this fee is the predominant one among all costs.

The cost can be described by:

costPerTx = gasL2 * gasPriceL2 + gasL1 * gasPriceL1 * feeScalar

Recent values seen in transactions were:

  • L1 Gas Used by Txn: ~2,600
  • L1 Fee Scalar: 0.684
  • L1 Gas Price: 55 Gwei
  • L1Fees = 9.7812e-05 ETH == ~0.19 USD

On the other hand, the L2 associated cost to execute the withdraw exploit were:

  • L2 Gas Used (mean during fuzzing): ~103,000
  • L2 Gas Price: 600-1000000
  • L2Fees (extreme lower bound) = 6.18e-11 ETH == ~1.23e-7 USD

The maximum value that can be withdrawn from the attack is ~10^10/10^18 WETH, which in FIAT values would be of 2e-5 USD. That means that the added cost to perform such an attack is significantly higher than the funds stolen from the protocol.

However, a malicious user might want to use a single transaction with multiple calls to the withdraw function to reduce the cost of publishing a single transaction in L1. A single block in L2 has enough space to accommodate roughly 290 calls to the withdraw function. In the ideal scenario that the malicious user would be able to create a contract that would call multiple times that function and use the entire block for themselves, the funds retrieved through the attack would be at most ~0.0058 USD (without the gas cost associated to both networks), which is still insufficient to break even with the cost of publishing the transaction. Only if in the extreme case the L1 gas price drops to the lower single digit values, the procedure will break even at the expense of using all the block with little to no reward.

The rate in which the malicious user can steal from the protocol, taking into account that the Base network mines a block every 2 seconds on average, that the block size is 30M, that the gas price in L2 has been seen around the 600 wei and the one in L1 is currently around 31 Gwei, and that the malicious actor must hijack the whole blocks to achieve an “efficient” execution (this means, publishing a transaction only once but doing as many withdraws as they can), some numbers that come from that are:

Exploit/hr [USD/Hr]: $0.0058 * (60 * 60) / 2 = $10.44/hr

Cost/hr (at current Ethereum mainnet values):

(30,000,000 * 600 / 10^18) * 2000 * (60 * 60) / 2

+ (2600 * 30 Gwei * 0.684 / 10^18) * 2000 * (60 * 60) /2

= ($0.0648 + $192.07)/Hr = $192.1348/hr

Cost/hr (at most favorable case of 1 Gwei in L1):

(30,000,000 * 600 / 10^18) * 2000 * (60 * 60) / 2

+ (2600 * 1 Gwei * 0.684 / 10^18) * 2000 * (60 * 60) /2

= ($0.0648 + $6.40)/Hr = $6.4648/hr

Is this issue exploitable in other ways?

The rounding issue is contained to the isBorrowCollateralized function which is called from the transfer and withdraw family of functions. There are execution paths that also use isBorrowCollateralized when withdrawing/transferring _collateral _assets, which was not part of the disclosure. So is it possible to craft a similar attack that withdraws collateral assets from the protocol? No. Collateral asset balances are stored as unsigned integers, meaning if you tried to subtract an amount from a position greater than is already there (e.g. 1e10 from 0), the calculation would underflow and revert. Therefore, you can only ever withdraw collateral that has been put onto the protocol. Trying to learn from this, one could ask if storing borrow and supply positions separately as unsigned integers could similarly shield the protocol but no. Such a system would require logic to handle withdrawing excess amounts so no underflows would occur as withdrawal amounts in excess of the supply position would simply be added to the borrow value and the rounding could still manifest.

What actions would mitigate this issue?

  1. Create a new hard-coded value in CometCore that is used at Comet construction to intelligently set the baseBorrowMin value such that the exploit is removed
  2. Within isBorrowCollateralized, short circuit the calculation to return false if the account’s assetsIn variable is zero
  3. Within isBorrowCollateralized change the final check on liquidity to be strictly greater than zero so any rounding issues return false.

Conclusion

Despite the economic infeasibility of this vulnerability, we must still take any edge case seriously, especially when they are technically able to break core invariants of the protocol. We’re very grateful to Britto for reporting the vulnerability to us and working with OpenZeppelin and Compound Labs to ensure the protocol is patched. We’re also grateful to Gauntlet for their support in fixing the issue on Comet WETH with their proposal.

Based on community feedback, this situation has also highlighted the need for a robust bug bounty program and we will be working to solicit potential vendors to approach the community with a more formalized program to better handle bug payout severity ranking and negotiations through a reputable third-party. Expect to hear more updates on this in the new year.

1 Like