No check to ensure that the orderParameters consideration array is not empty

[code423n4]

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/Assertions.sol#L50

Vulnerability details

Impact

In the _assertConsiderationLengthAndGetOrderHash function, there is no check to ensure that the orderParameters consideration array is not empty. This could result in an out of bounds array access error when trying to retrieve the length of the consideration array.

Proof of Concept

Let's say we have the following order parameters:

OrderParameters memory orderParameters = OrderParameters({

offerer: address(0x1234),

consideration: [],

totalOriginalConsiderationItems: 2

});

Now, when we call the _assertConsiderationLengthAndGetOrderHash function with these order parameters:

_assertConsiderationLengthAndGetOrderHash(orderParameters);

The function will try to retrieve the length of the consideration array by calling orderParameters.consideration.length which is 0 in this case, but the totalOriginalConsiderationItems is 2, which means that the function will raise an error because the supplied consideration array length is less than the original. However, the array is empty and the orderParameters.consideration.length will return 0 which will be less than totalOriginalConsiderationItems which is 2, this will trigger the _revertMissingOriginalConsiderationItems() function and revert the transaction which is not expected.

Tools Used

vs code

Recommended Mitigation Steps

To solve this issue, the function should check if the consideration array is empty before comparing its length with the totalOriginalConsiderationItems

[0age]

contested; appears to be a misunderstanding of how the encoding works, an offset and length are still provided even on empty arrays

[HickupHH3]

this will trigger the _revertMissingOriginalConsiderationItems() function and revert the transaction which is not expected.

It's 100% expected, no? Empty array length = 0, totalOriginalConsiderationItems = 2, condition fulfilled for revert (assertion fails), so... revert?

 function _assertConsiderationLengthIsNotLessThanOriginalConsiderationLength(
  uint256 suppliedConsiderationItemTotal,
  uint256 originalConsiderationItemTotal
) internal pure {
  // Ensure supplied consideration array length is not less than original.
  if (suppliedConsiderationItemTotal < originalConsiderationItemTotal) {
    _revertMissingOriginalConsiderationItems();
  }
}

[c4-judge]

HickupHH3 marked the issue as unsatisfactory: Invalid