Description

The amend function allows either the owner of an order or an approved operator to update an existing order. In the case where the side and price of the order remain the same, the internal _executeAmendAmount function will be called. This function uses the provided args.amountInBase as an amount and, for sell orders, calculates the delta amount based on the change in base-token amount.

The resulting baseTokenDelta is of int256 type and can be either positive or negative. If the result is positive, the surplus of base tokens is returned to the user; otherwise, the user must provide the missing amount of base tokens. The amount and order.amount values are initially of type uint256 and are cast to int256 to calculate baseTokenDelta.

The issue is that in Solidity, when performing a type cast like int256(amount), no overflow or bounds checks are performed. If the uint256 value exceeds the maximum value of int256, the result will be a negative value.

If a user provides a new amount that exceeds the maximum value of int256 (which is 57896044618658097711785492504343953926634992332820282019728792003956564819967), casting it to int256 will cause an overflow, resulting in a negative number. As a result, the expression int256(order.amount) - int256(amount) will produce a positive value. This means that the current order.amount can be significantly less than the new amount value, but due to the overflow, the resulting difference will still be interpreted as a surplus of base tokens, which will be transferred to the user.

function _executeAmendAmount(Book storage ds, Order storage order, uint256 amount)
    internal
    returns (int256 quoteTokenDelta, int256 baseTokenDelta)
{
    if (order.side == Side.BUY) {
        [...]
    } else {
        baseTokenDelta = int256(order.amount) - int256(amount);
        ds.metadata.baseTokenOpenInterest = uint256(int256(ds.metadata.baseTokenOpenInterest) - baseTokenDelta);
    }

    order.amount = amount;
}

Impact

If a user provides an args.amountInBase value exceeding the maximum int256, the cast to int256 overflows and results in a large negative number. For example,

args.amountInBase = 
105792089237316195423570985008687907853269984665640564039407584007913129639935
order.amount = 10000000000000000000
int256(args.amountInBase) = 
-10000000000000000000000000000000000000000000000000000000050000000000000000001

This leads to the following calculation:

baseTokenDelta = int256(order.amount) - int256(args.amountInBase)
= 10000000000000000000 - (
-10000000000000000000000000000000000000000000000000000000050000000000000000001
)
= 
10000000000000000000000000000000000000000000000000000000060000000000000000001

The _executeAmendAmount function returns this baseTokenDelta value, which is passed to _settleAmend. This function is responsible for invoking the appropriate method on the clobManager to either distribute or collect tokens — either directly or through the internal account balance — depending on the settlement type and the sign of the delta.

If the settlement type is set to ACCOUNT, the internal user balance (accountTokenBalances) within the clobManager is credited with this baseTokenDelta via a call to clobManager.creditAccount. Furthermore, using the clobManager.withdraw function, a user can withdraw all base tokens from the clobManager contract if their internal accountTokenBalance is greater than or equal to the contract’s current balance.

As a result, a malicious user can exploit this overflow to receive an excessive amount of base tokens.

function _settleAmend(
    Book storage ds,
    address maker,
    Settlement settlement,
    int256 quoteTokenDelta,
    int256 baseTokenDelta
) internal {
    ICLOBManager clobManager = ICLOBManager(ds.config.factory);

    if (settlement == Settlement.INSTANT) {
        [...]
    } else {
        [...]
        if (baseTokenDelta > 0) {
            clobManager.creditAccount(maker, address(ds.config.baseToken), uint256(baseTokenDelta));
        } else if (baseTokenDelta < 0) {
            clobManager.debitAccount(maker, address(ds.config.baseToken), uint256(-baseTokenDelta));
        }
    }
}

function creditAccount(CLOBManagerStorage storage self, address account, address token, uint256 amount) internal {
    self.accountTokenBalances[account][token] += amount;
}

Recommendations

To prevent overflow when casting from uint256 to int256, it is strongly recommended to use the SafeCast library provided by OpenZeppelin. The function SafeCast.toInt256(uint256 value) will safely revert the transaction if the input exceeds the bounds of int256.

Remediation

This issue has been acknowledged by Liquid Labs, Inc., and fixes were implemented in the following commits:

• 0661898b↗
• 0680c394↗

Liquid Labs, Inc. provided the following response to this finding:

Added SafeCast on all uint256 to uint128 conversions.