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

1 stars 0 forks source link

Batch transfer does not check if `ids` and `amounts` array lengths are equal #24

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/TokenTransferrer.sol#L476 https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/lib/ConduitStructs.sol#L19-L20

Vulnerability details

Impact

In TokenTransferrer.sol the _performERC1155BatchTransfers() function takes in an array of transfers of type ConduitBatch1155Transfer. The ConduitBatch1155Transfer struct contains the ids and amounts arrays. The problem is that when iterating over the batchTransfers the _performERC1155BatchTransfers() function does not check if the ids and amounts arrays are the same length. This can result in the function failing dues to array of bounds errors.

Proof of Concept

https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/TokenTransferrer.sol#L476

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/lib/ConduitStructs.sol#L19-L20

Tools Used

Manual code review

Recommended Mitigation Steps

The _performERC1155BatchTransfers() function should fail if the ids and amounts arrays are not the same length when iterating over batchTransfer.

d1ll0n commented 2 years ago

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/TokenTransferrer.sol#L572

It does check this

0xleastwood commented 2 years ago

As the sponsor mentioned, this is already checked. Therefore, this issue is invalid.