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

1 stars 0 forks source link

QA Report #145

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Vulnerability details:

Summary

Low Risk Issues

Issue Instances
1 Use of transferFrom() rather than safeTransferFrom() for NFTs in the reference code will lead to the loss of NFTs 1
2 Holdings not verified 1
3 Lack of escrow will lead to a lot of spoofing 1
4 Incorrect NatSpec 1
5 Reference code signatures can't work with optimized code 1
6 Misleading comments 1
7 Incomplete code cleanup 1
8 The unchecked keyword is for mathematical impossibilities, not assumptions 1
9 'ERC20' tokens with incorrect return types that are over-sized, are interpreted as returning bool 1
10 Missing fill-or-kill behavior 1
11 abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() 7

Total: 17 instances over 11 issues

Non-critical Issues

Issue Instances
1 Unused file 1
2 Adding a return statement when the function defines a named return variable, is redundant 9
3 constants should be defined rather than using magic numbers 41
4 Use a more recent version of solidity 6
5 Inconsistent spacing in comments 1
6 Inconsistent method of specifying a floating pragma 22
7 Non-library/interface files should use fixed compiler versions, not floating ones 20
8 Inconsistent code formatting 6
9 Inconsistent use of named arguments within interfaces in same file 2
10 Duplicated code 1
11 Anyone should be able to cancel an expired order 1
12 Typos 7
13 File is missing NatSpec 8
14 NatSpec is incomplete 13
15 Event is missing indexed fields 6
16 Not using the named return variables anywhere in the function is confusing 46

Total: 190 instances over 16 issues

Low Risk Issues

1. Use of transferFrom() rather than safeTransferFrom() for NFTs in the reference code will lead to the loss of NFTs

See the Medium issue I filed for the non-reference, optimized code for details

There is 1 instance of this issue:

File: reference/lib/ReferenceTokenTransferrer.sol   #1

82:           ERC721Interface(token).transferFrom(from, to, identifier);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceTokenTransferrer.sol#L82

2. Holdings not verified

During offer creation, there is no code that verifies that the user actually holds the items they're offering (e.g. balanceOf(). The code should make some sort of attempt to verify holdings before accepting an order, to cut down on the amount of spoofing possible

There is 1 instance of this issue:

File: contracts/lib/Consideration.sol   #1

174       *         of items for offer and consideration. Any order that is not
175       *         currently active, has already been fully filled, or has been
176:      *         cancelled will be omitted. Remaining offer and consideration

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

3. Lack of escrow will lead to a lot of spoofing

The average user will not be able to simulate whether an order will be fulfillable or not, and even if it is at that exact moment, without an escrow, the seller can transfer funds, cancel, or front-run at any time. There should be an escrow option so that users have a way to know that offers are real. This was found to be of Medium risk in a recent contest

There is 1 instance of this issue:

File: contracts/lib/Consideration.sol   #1

174       *         of items for offer and consideration. Any order that is not
175       *         currently active, has already been fully filled, or has been
176:      *         cancelled will be omitted. Remaining offer and consideration

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

4. Incorrect NatSpec

The qualification in parentheses should have been added to the amount parameter, not the identifier parameter

There is 1 instance of this issue:

File: contracts/lib/Executor.sol   #1

301       * @param identifier  The tokenId to transfer (must be 1 for ERC721).
302:      * @param amount      The amount to transfer.

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

5. Reference code signatures can't work with optimized code

The reference code uses "rc.1" in its construction of the domain separator, whereas the actual code uses "1"

There is 1 instance of this issue:

File: reference/lib/ReferenceConsiderationBase.sol   #1

30:      string internal constant _VERSION = "rc.1";

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationBase.sol#L30

6. Misleading comments

The code checks the first twenty bytes, not the last

There is 1 instance of this issue:

File: contracts/interfaces/ConduitControllerInterface.sol   #1

111       *                     the last twenty bytes of the conduit key must match
112:      *                     the caller of this contract.

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConduitControllerInterface.sol#L111-L112

7. Incomplete code cleanup

It seems as though the plan originally was to use this interface for doing create2, but later things switched to using a salt with new, and this interface wasn't moved to be test-only. If the above is right, you should see if anything during that transition was also missed

There is 1 instance of this issue:

File: contracts/interfaces/ImmutableCreate2FactoryInterface.sol   #1

18:  interface ImmutableCreate2FactoryInterface {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ImmutableCreate2FactoryInterface.sol#L18

8. The unchecked keyword is for mathematical impossibilities, not assumptions

It's an assumption that the nonce cannot be incremented that far, and therfore the use of unchecked is invalid

There is 1 instance of this issue:

File: contracts/lib/NonceManager.sol   #1

32           // No need to check for overflow; nonce cannot be incremented that far.
33           unchecked {
34               // Increment current nonce for the supplied offerer.
35               newNonce = ++_nonces[msg.sender];
36:          }

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

9. 'ERC20' tokens with incorrect return types that are over-sized, are interpreted as returning bool

If there's a token that has the wrong return type, e.g. returns (bool,bool), the code below will mload and check the value of the first bool, and will ignore the fact that there is extra return data. Th code should change to be eq(returndatasize(), 32). The reference code has the same issue

There is 1 instance of this issue:

File: contracts/lib/TokenTransferrer.sol   #1

41               // Make call & copy up to 32 bytes of return data to scratch space.
42               let callStatus := call(
43                   gas(),
44                   token,
45                   0,
46                   ERC20_transferFrom_sig_ptr,
47                   ERC20_transferFrom_length,
48                   0,
49                   OneWord
50               )
51   
52               // Determine whether transfer was successful using status & result.
53               let success := and(
54                   // Set success to whether the call reverted, if not check it
55                   // either returned exactly 1 (can't just be non-zero data), or
56                   // had no return data.
57                   or(
58:                      and(eq(mload(0), 1), gt(returndatasize(), 31)),

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

10. Missing fill-or-kill behavior

A useful order type for traders is the fill-or-kill order type, and by requiring that the ending timestamp be greater than the current timestamp, you are unnecessarily preventing this order type

There is 1 instance of this issue:

File: contracts/lib/Verifiers.sol   #1

37       function _verifyTime(
38           uint256 startTime,
39           uint256 endTime,
40           bool revertOnInvalid
41       ) internal view returns (bool valid) {
42           // Revert if order's timespan hasn't started yet or has already ended.
43:          if (startTime > block.timestamp || endTime <= block.timestamp) {

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

11. abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.

If all arguments are strings and or bytes, bytes.concat() should be used instead

There are 7 instances of this issue:

File: contracts/lib/ConsiderationBase.sol

193           eip712DomainTypehash = keccak256(
194               abi.encodePacked(
195                   "EIP712Domain(",
196                       "string name,",
197                       "string version,",
198                       "uint256 chainId,",
199                       "address verifyingContract",
200                   ")"
201               )
202:          );

211           orderTypehash = keccak256(
212               abi.encodePacked(
213                   orderComponentsPartialTypeString,
214                   considerationItemTypeString,
215                   offerItemTypeString
216               )
217:          );

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

File: reference/lib/ReferenceGettersAndDerivers.sol

129:                      keccak256(abi.encodePacked(offerHashes)),

130:                      keccak256(abi.encodePacked(considerationHashes)),

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L129

File: reference/lib/ReferenceBasicOrderFulfiller.sol

769               hashes.offerItemsHash = keccak256(
770                   abi.encodePacked(offerItemHashes)
771:              );

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L769-L771

File: reference/lib/ReferenceConsiderationBase.sol

207           eip712DomainTypehash = keccak256(
208               abi.encodePacked(
209                   "EIP712Domain(",
210                       "string name,",
211                       "string version,",
212                       "uint256 chainId,",
213                       "address verifyingContract",
214                   ")"
215               )
216:          );

225           orderTypehash = keccak256(
226               abi.encodePacked(
227                   orderComponentsPartialTypeString,
228                   considerationItemTypeString,
229                   offerItemTypeString
230               )
231:          );

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationBase.sol#L207-L216

Non-critical Issues

1. Unused file

The file is never imported by any other file. After deleting the file, I was still able to run the hardhat tests, so it seems as though this file is no longer needed.

There is 1 instance of this issue:

File: reference/shim/Shim.sol   #1

0:    // SPDX-License-Identifier: MIT

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/shim/Shim.sol#L0

2. Adding a return statement when the function defines a named return variable, is redundant

There are 9 instances of this issue:

File: contracts/lib/FulfillmentApplier.sol

120:          return execution; // Execution(considerationItem, offerer, conduitKey);

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

File: contracts/lib/OrderFulfiller.sol

478:          return advancedOrders;

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

File: reference/lib/ReferenceOrderCombiner.sol

655:          return availableOrders;

789:          return executions;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L655

File: reference/lib/ReferenceFulfillmentApplier.sol

128:          return execution; // Execution(considerationItem, offerer, conduitKey);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L128

File: reference/lib/ReferenceOrderFulfiller.sol

299:          return orderToExecute;

351:          return advancedOrders;

427:          return orderToExecute;

454:          return ordersToExecute;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderFulfiller.sol#L299

3. constants should be defined rather than using magic numbers

Normally I wouldn't have flagged hex values, but it looks like in other places you've created constants for them, so I've flagged all missing places

There are 41 instances of this issue:

File: contracts/conduit/ConduitController.sol

/// @audit 0xff
74:                               bytes1(0xff),

/// @audit 0xff
326:                              bytes1(0xff),

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L74

File: contracts/lib/CriteriaResolution.sol

/// @audit 0x20
268:                      mstore(0x20, loadedData) // Place new hash next.

/// @audit 0x20
272:                      mstore(0x20, computedHash) // Place existing hash next.

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

File: contracts/lib/SignatureVerification.sol

/// @audit 64
45:           if (signature.length == 64) {

/// @audit 65
63:           } else if (signature.length == 65) {

/// @audit 27
78:               if (v != 27 && v != 28) {

/// @audit 28
78:               if (v != 27 && v != 28) {

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

File: contracts/lib/Executor.sol

/// @audit 3
387:                  uint256(3),

/// @audit 64
433:          if (accumulator.length != 64) {

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

File: contracts/lib/GettersAndDerivers.sol

/// @audit 0xf8
322:              mstore(add(version, OneWord), shl(0xf8, Version))

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

File: contracts/Seaport.sol

/// @audit 0x20
40:               mstore(0, 0x20)

/// @audit 0x27
41:               mstore(0x27, 0x07536561706f7274)

/// @audit 0x07536561706f7274
41:               mstore(0x27, 0x07536561706f7274)

/// @audit 0x60
42:               return(0, 0x60)

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

File: reference/conduit/ReferenceConduitController.sol

/// @audit 0xff
76:                               bytes1(0xff),

/// @audit 0xff
332:                              bytes1(0xff),

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduitController.sol#L76

File: reference/lib/ReferenceSignatureVerification.sol

/// @audit 64
43:           if (signature.length == 64) {

/// @audit 255
51:               v = uint8(uint256(vs >> 255)) + 27;

/// @audit 27
51:               v = uint8(uint256(vs >> 255)) + 27;

/// @audit 65
52:           } else if (signature.length == 65) {

/// @audit 64
54:               v = uint8(signature[64]);

/// @audit 27
57:               if (v != 27 && v != 28) {

/// @audit 28
57:               if (v != 27 && v != 28) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceSignatureVerification.sol#L43

File: reference/lib/ReferenceExecutor.sol

/// @audit 3
269:                  uint256(3),

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceExecutor.sol#L269

File: reference/lib/ReferenceGettersAndDerivers.sol

/// @audit 0x1901
157:              abi.encodePacked(uint16(0x1901), domainSeparator, orderHash)

/// @audit 0xff
184:                              bytes1(0xff),

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L157

File: reference/lib/ReferenceCriteriaResolution.sol

/// @audit 3
358:          withCriteria = uint256(itemType) > 3;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceCriteriaResolution.sol#L358

File: reference/lib/ReferenceTokenTransferrer.sol

/// @audit 32
50:           if (data.length != 0 && data.length >= 32) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceTokenTransferrer.sol#L50

File: reference/lib/ReferenceAssertions.sol

/// @audit 4
112:          bool validOffsets = (abi.decode(msg.data[4:36], (uint256)) == 32 &&

/// @audit 36
112:          bool validOffsets = (abi.decode(msg.data[4:36], (uint256)) == 32 &&

/// @audit 32
112:          bool validOffsets = (abi.decode(msg.data[4:36], (uint256)) == 32 &&

/// @audit 548
113:              abi.decode(msg.data[548:580], (uint256)) == 576 &&

/// @audit 580
113:              abi.decode(msg.data[548:580], (uint256)) == 576 &&

/// @audit 576
113:              abi.decode(msg.data[548:580], (uint256)) == 576 &&

/// @audit 580
114:              abi.decode(msg.data[580:612], (uint256)) ==

/// @audit 612
114:              abi.decode(msg.data[580:612], (uint256)) ==

/// @audit 608
115:              608 + 64 * abi.decode(msg.data[612:644], (uint256)));

/// @audit 64
115:              608 + 64 * abi.decode(msg.data[612:644], (uint256)));

/// @audit 612
115:              608 + 64 * abi.decode(msg.data[612:644], (uint256)));

/// @audit 644
115:              608 + 64 * abi.decode(msg.data[612:644], (uint256)));

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceAssertions.sol#L112

4. Use a more recent version of solidity

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)

There are 6 instances of this issue:

File: contracts/conduit/ConduitController.sol

2:    pragma solidity >=0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L2

File: reference/conduit/ReferenceConduitController.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduitController.sol#L2

File: reference/lib/ReferenceGettersAndDerivers.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L2

File: reference/lib/ReferenceBasicOrderFulfiller.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L2

File: reference/lib/ReferenceCriteriaResolution.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceCriteriaResolution.sol#L2

File: reference/lib/ReferenceConsiderationBase.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationBase.sol#L2

5. Inconsistent spacing in comments

Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file

There is 1 instance of this issue:

File: reference/lib/ReferenceBasicOrderFulfiller.sol   #1

763:                          offerItem.amount //Assembly uses OfferItem instead of SpentItem.

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L763

6. Inconsistent method of specifying a floating pragma

Some files use >=, some use ^. The instances below are examples of the method that has the fewest instances for a specific version. Note that using >= without also specifying <= will lead to failures to compile when the major version changes and there are breaking-changes, so ^ should be preferred regardless of the instance counts

There are 22 instances of this issue:

File: reference/conduit/ReferenceConduitController.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduitController.sol#L2

File: reference/conduit/ReferenceConduit.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduit.sol#L2

File: reference/lib/ReferenceSignatureVerification.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceSignatureVerification.sol#L2

File: reference/lib/ReferenceExecutor.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceExecutor.sol#L2

File: reference/lib/ReferenceOrderCombiner.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L2

File: reference/lib/ReferenceFulfillmentApplier.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L2

File: reference/lib/ReferenceOrderValidator.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderValidator.sol#L2

File: reference/lib/ReferenceZoneInteraction.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceZoneInteraction.sol#L2

File: reference/lib/ReferenceGettersAndDerivers.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L2

File: reference/lib/ReferenceAmountDeriver.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceAmountDeriver.sol#L2

File: reference/lib/ReferenceOrderFulfiller.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderFulfiller.sol#L2

File: reference/lib/ReferenceBasicOrderFulfiller.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L2

File: reference/lib/ReferenceNonceManager.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceNonceManager.sol#L2

File: reference/lib/ReferenceCriteriaResolution.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceCriteriaResolution.sol#L2

File: reference/lib/ReferenceReentrancyGuard.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceReentrancyGuard.sol#L2

File: reference/lib/ReferenceVerifiers.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceVerifiers.sol#L2

File: reference/lib/ReferenceConsiderationStructs.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationStructs.sol#L2

File: reference/lib/ReferenceTokenTransferrer.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceTokenTransferrer.sol#L2

File: reference/lib/ReferenceConsiderationBase.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationBase.sol#L2

File: reference/lib/ReferenceAssertions.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceAssertions.sol#L2

File: reference/shim/Shim.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/shim/Shim.sol#L2

File: reference/ReferenceConsideration.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/ReferenceConsideration.sol#L2

7. Non-library/interface files should use fixed compiler versions, not floating ones

There are 20 instances of this issue:

File: reference/conduit/ReferenceConduitController.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduitController.sol#L2

File: reference/conduit/ReferenceConduit.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduit.sol#L2

File: reference/lib/ReferenceSignatureVerification.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceSignatureVerification.sol#L2

File: reference/lib/ReferenceExecutor.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceExecutor.sol#L2

File: reference/lib/ReferenceOrderCombiner.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L2

File: reference/lib/ReferenceFulfillmentApplier.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L2

File: reference/lib/ReferenceOrderValidator.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderValidator.sol#L2

File: reference/lib/ReferenceZoneInteraction.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceZoneInteraction.sol#L2

File: reference/lib/ReferenceGettersAndDerivers.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L2

File: reference/lib/ReferenceAmountDeriver.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceAmountDeriver.sol#L2

File: reference/lib/ReferenceOrderFulfiller.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderFulfiller.sol#L2

File: reference/lib/ReferenceBasicOrderFulfiller.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L2

File: reference/lib/ReferenceNonceManager.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceNonceManager.sol#L2

File: reference/lib/ReferenceCriteriaResolution.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceCriteriaResolution.sol#L2

File: reference/lib/ReferenceReentrancyGuard.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceReentrancyGuard.sol#L2

File: reference/lib/ReferenceVerifiers.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceVerifiers.sol#L2

File: reference/lib/ReferenceTokenTransferrer.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceTokenTransferrer.sol#L2

File: reference/lib/ReferenceConsiderationBase.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationBase.sol#L2

File: reference/lib/ReferenceAssertions.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceAssertions.sol#L2

File: reference/ReferenceConsideration.sol

2:    pragma solidity ^0.8.7;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/ReferenceConsideration.sol#L2

8. Inconsistent code formatting

In most places throughout the code, external, payable, and returns (<x>) each appear on their respective own lines when the full function signature doesn't fit within 80 characters. The examples below don't follow this pattern

There are 6 instances of this issue:

File: contracts/interfaces/ConsiderationInterface.sol

118:     ) external payable returns (bool fulfilled);

273:     ) external payable returns (Execution[] memory executions);

315:     ) external payable returns (Execution[] memory executions);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConsiderationInterface.sol#L118

File: contracts/interfaces/SeaportInterface.sol

117:     ) external payable returns (bool fulfilled);

272:     ) external payable returns (Execution[] memory executions);

314:     ) external payable returns (Execution[] memory executions);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/SeaportInterface.sol#L117

9. Inconsistent use of named arguments within interfaces in same file

The examples below have functions where there are no named arguments. The other interfaces defined in this file are pretty similar, but have named arguments, as is the pattern throughout the rest of the interfaces in the rest of the files

There are 2 instances of this issue:

File: contracts/interfaces/AbridgedTokenInterfaces.sol   #1

5        function transferFrom(
6            address,
7            address,
8            uint256
9:       ) external returns (bool);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/AbridgedTokenInterfaces.sol#L5-L9

File: contracts/interfaces/AbridgedTokenInterfaces.sol   #2

13       function transferFrom(
14           address,
15           address,
16           uint256
17:      ) external;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/AbridgedTokenInterfaces.sol#L13-L17

10. Duplicated code

The code block below is copy-pasted from here with only minor modifications. The code should be refactored to a common function that takes in an array and does the transfers. The non-reference version of the file has the same issue, but I'm assuming that you'd want that one to stay as-is to avoid the extra JUMP associated with a function call

There is 1 instance of this issue:

File: reference/conduit/ReferenceConduit.sol   #1

88           uint256 totalStandardTransfers = standardTransfers.length;
89   
90           // Iterate over each standard transfer.
91           for (uint256 i = 0; i < totalStandardTransfers; i++) {
92               // Retrieve the transfer in question.
93               ConduitTransfer calldata standardTransfer = standardTransfers[i];
94   
95               // Perform the transfer.
96               _transfer(standardTransfer);
97:          }

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduit.sol#L88-L97

11. Anyone should be able to cancel an expired order

Having orders live around forever makes off-chain tracking via events, more difficult. By allowing anyone to cancel orders whose end time has passed, will allow some orders to be cleaned up

There is 1 instance of this issue:

File: contracts/lib/Consideration.sol   #1

413       * @notice Cancel an arbitrary number of orders. Note that only the offerer
414       *         or the zone of a given order may cancel it. Callers should ensure
415       *         that the intended order was cancelled by calling `getOrderStatus`
416       *         and confirming that `isCancelled` returns `true`.
417       *
418       * @param orders The orders to cancel.
419       *
420       * @return cancelled A boolean indicating whether the supplied orders have
421       *                   been successfully cancelled.
422       */
423:     function cancel(OrderComponents[] calldata orders)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/7c082637d9273deb42e551c74a578151cefd8448/contracts/lib/Consideration.sol#L413-L423

12. Typos

There are 7 instances of this issue:

File: contracts/lib/FulfillmentApplier.sol

/// @audit receipient
180:              // Set the offerer as the receipient if execution amount is nonzero.

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

File: contracts/lib/Assertions.sol

/// @audit amont
92:           // Revert if the supplied amont is equal to zero.

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

File: contracts/interfaces/SeaportInterface.sol

/// @audit toreceive
287:       *                          order toreceive ERC1155 tokens. Also note that

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/SeaportInterface.sol#L287

File: contracts/interfaces/ZoneInteractionErrors.sol

/// @audit offerrer
13:        *      either the offerrer or the order's zone or approved as valid by the

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ZoneInteractionErrors.sol#L13

File: contracts/interfaces/TokenTransferrerErrors.sol

/// @audit falsey
56:        * @dev Revert with an error when an ERC20 token transfer returns a falsey

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/TokenTransferrerErrors.sol#L56

File: contracts/interfaces/ConsiderationInterface.sol

/// @audit toreceive
288:       *                          order toreceive ERC1155 tokens. Also note that

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConsiderationInterface.sol#L288

File: reference/lib/ReferenceExecutor.sol

/// @audit invalud
436:              // Revert with an error indicating an invalud conduit.

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceExecutor.sol#L436

13. File is missing NatSpec

There are 8 instances of this issue:

File: contracts/conduit/lib/ConduitEnums.sol

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

File: contracts/conduit/lib/ConduitStructs.sol

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

File: contracts/lib/ConsiderationConstants.sol

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

File: contracts/lib/ConsiderationEnums.sol

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

File: contracts/lib/TokenTransferrerConstants.sol

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

File: contracts/interfaces/ZoneInterface.sol

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ZoneInterface.sol

File: contracts/interfaces/AbridgedTokenInterfaces.sol

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/AbridgedTokenInterfaces.sol

File: contracts/interfaces/EIP1271Interface.sol

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/EIP1271Interface.sol

14. NatSpec is incomplete

There are 13 instances of this issue:

File: contracts/conduit/ConduitController.sol

/// @audit Missing: '@param newPotentialOwner'
181       /**
182        * @notice Initiate conduit ownership transfer by assigning a new potential
183        *         owner for the given conduit. Once set, the new potential owner
184        *         may call `acceptOwnership` to claim ownership of the conduit.
185        *         Only the owner of the conduit in question may call this function.
186        *
187        * @param conduit The conduit for which to initiate ownership transfer.
188        */
189:      function transferOwnership(address conduit, address newPotentialOwner)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L181-L189

File: contracts/lib/BasicOrderFulfiller.sol

/// @audit Missing: '@param accumulator'
1001      /**
1002       * @dev Internal function to transfer ERC20 tokens to a given recipient as
1003       *      part of basic order fulfillment.
1004       *
1005       * @param from                 The originator of the ERC20 token transfer.
1006       * @param to                   The recipient of the ERC20 token transfer.
1007       * @param erc20Token           The ERC20 token to transfer.
1008       * @param amount               The amount of ERC20 tokens to transfer.
1009       * @param additionalRecipients The additional recipients of the order.
1010       * @param fromOfferer          A boolean indicating whether to decrement
1011       *                             amount from the offered amount.
1012       */
1013      function _transferERC20AndFinalize(
1014          address from,
1015          address to,
1016          address erc20Token,
1017          uint256 amount,
1018          AdditionalRecipient[] calldata additionalRecipients,
1019          bool fromOfferer,
1020:         bytes memory accumulator

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

File: contracts/lib/AmountDeriver.sol

/// @audit Missing: '@param roundUp'
123       /**
124        * @dev Internal pure function to apply a fraction to a consideration
125        * or offer item.
126        *
127        * @param startAmount     The starting amount of the item.
128        * @param endAmount       The ending amount of the item.
129        * @param numerator       A value indicating the portion of the order that
130        *                        should be filled.
131        * @param denominator     A value indicating the total size of the order.
132        * @param elapsed         The time elapsed since the order's start time.
133        * @param remaining       The time left until the order's end time.
134        * @param duration        The total duration of the order.
135        *
136        * @return amount The received item to transfer with the final amount.
137        */
138       function _applyFraction(
139           uint256 startAmount,
140           uint256 endAmount,
141           uint256 numerator,
142           uint256 denominator,
143           uint256 elapsed,
144           uint256 remaining,
145           uint256 duration,
146           bool roundUp
147:      ) internal pure returns (uint256 amount) {

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

File: contracts/interfaces/ConduitControllerInterface.sol

/// @audit Missing: '@param newPotentialOwner'
139       /**
140        * @notice Initiate conduit ownership transfer by assigning a new potential
141        *         owner for the given conduit. Once set, the new potential owner
142        *         may call `acceptOwnership` to claim ownership of the conduit.
143        *         Only the owner of the conduit in question may call this function.
144        *
145        * @param conduit The conduit for which to initiate ownership transfer.
146        */
147:      function transferOwnership(address conduit, address newPotentialOwner)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConduitControllerInterface.sol#L139-L147

File: reference/conduit/ReferenceConduitController.sol

/// @audit Missing: '@param newPotentialOwner'
184       /**
185        * @dev External function for initiating conduit ownership transfer by
186        *      assigning a new potential owner for the given conduit. Once set, the
187        *      new potential owner may call `acceptOwnership` to claim ownership of
188        *      the conduit. Only the owner of the conduit in question may call this
189        *      function.
190        *
191        * @param conduit The conduit for which to initiate ownership transfer.
192        */
193:      function transferOwnership(address conduit, address newPotentialOwner)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduitController.sol#L184-L193

File: reference/lib/ReferenceAmountDeriver.sol

/// @audit Missing: '@param roundUp'
103       /**
104        * @dev Internal pure function to apply a fraction to a consideration
105        * or offer item.
106        *
107        * @param startAmount     The starting amount of the item.
108        * @param endAmount       The ending amount of the item.
109        * @param fractionData    A struct containing the data used to apply a
110        *                        fraction to an order.
111        * @return amount The received item to transfer with the final amount.
112        */
113       function _applyFraction(
114           uint256 startAmount,
115           uint256 endAmount,
116           FractionData memory fractionData,
117           bool roundUp
118:      ) internal pure returns (uint256 amount) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceAmountDeriver.sol#L103-L118

File: reference/lib/ReferenceBasicOrderFulfiller.sol

/// @audit Missing: '@return'
502        * @param fulfillmentItemTypes The fulfillment's item type.
503        */
504       function _hashOrder(
505           BasicFulfillmentHashes memory hashes,
506           BasicOrderParameters calldata parameters,
507           FulfillmentItemTypes memory fulfillmentItemTypes
508:      ) internal view returns (bytes32 orderHash) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L502-L508

File: reference/lib/ReferenceConsiderationBase.sol

/// @audit Missing: '@param _eip712DomainTypeHash'
87        /**
88         * @dev Internal view function to derive the initial EIP-712 domain
89         *      separator.
90         *
91         * @return The derived domain separator.
92         */
93        function _deriveInitialDomainSeparator(
94            bytes32 _eip712DomainTypeHash,
95            bytes32 _nameHash,
96            bytes32 _versionHash
97:       ) internal view virtual returns (bytes32) {

/// @audit Missing: '@param _nameHash'
87        /**
88         * @dev Internal view function to derive the initial EIP-712 domain
89         *      separator.
90         *
91         * @return The derived domain separator.
92         */
93        function _deriveInitialDomainSeparator(
94            bytes32 _eip712DomainTypeHash,
95            bytes32 _nameHash,
96            bytes32 _versionHash
97:       ) internal view virtual returns (bytes32) {

/// @audit Missing: '@param _versionHash'
87        /**
88         * @dev Internal view function to derive the initial EIP-712 domain
89         *      separator.
90         *
91         * @return The derived domain separator.
92         */
93        function _deriveInitialDomainSeparator(
94            bytes32 _eip712DomainTypeHash,
95            bytes32 _nameHash,
96            bytes32 _versionHash
97:       ) internal view virtual returns (bytes32) {

/// @audit Missing: '@param _eip712DomainTypeHash'
106       /**
107        * @dev Internal view function to derive the EIP-712 domain separator.
108        *
109        * @return The derived domain separator.
110        */
111       function _deriveDomainSeparator(
112           bytes32 _eip712DomainTypeHash,
113           bytes32 _nameHash,
114           bytes32 _versionHash
115:      ) internal view virtual returns (bytes32) {

/// @audit Missing: '@param _nameHash'
106       /**
107        * @dev Internal view function to derive the EIP-712 domain separator.
108        *
109        * @return The derived domain separator.
110        */
111       function _deriveDomainSeparator(
112           bytes32 _eip712DomainTypeHash,
113           bytes32 _nameHash,
114           bytes32 _versionHash
115:      ) internal view virtual returns (bytes32) {

/// @audit Missing: '@param _versionHash'
106       /**
107        * @dev Internal view function to derive the EIP-712 domain separator.
108        *
109        * @return The derived domain separator.
110        */
111       function _deriveDomainSeparator(
112           bytes32 _eip712DomainTypeHash,
113           bytes32 _nameHash,
114           bytes32 _versionHash
115:      ) internal view virtual returns (bytes32) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationBase.sol#L87-L97

15. Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

There are 6 instances of this issue:

File: contracts/interfaces/ConsiderationEventsAndErrors.sol

25        event OrderFulfilled(
26            bytes32 orderHash,
27            address indexed offerer,
28            address indexed zone,
29            address fulfiller,
30            SpentItem[] offer,
31            ReceivedItem[] consideration
32:       );

41        event OrderCancelled(
42            bytes32 orderHash,
43            address indexed offerer,
44            address indexed zone
45:       );

56        event OrderValidated(
57            bytes32 orderHash,
58            address indexed offerer,
59            address indexed zone
60:       );

68:       event NonceIncremented(uint256 newNonce, address indexed offerer);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConsiderationEventsAndErrors.sol#L25-L32

File: contracts/interfaces/ConduitInterface.sol

48:       event ChannelUpdated(address channel, bool open);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConduitInterface.sol#L48

File: contracts/interfaces/ConduitControllerInterface.sol

29:       event NewConduit(address conduit, bytes32 conduitKey);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConduitControllerInterface.sol#L29

16. Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

There are 46 instances of this issue:

File: contracts/lib/OrderCombiner.sol

/// @audit executions
704       function _matchAdvancedOrders(
705           AdvancedOrder[] memory advancedOrders,
706           CriteriaResolver[] memory criteriaResolvers,
707           Fulfillment[] calldata fulfillments
708:      ) internal returns (Execution[] memory executions) {

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

File: contracts/lib/OrderValidator.sol

/// @audit newNumerator
104       function _validateOrderAndUpdateStatus(
105           AdvancedOrder memory advancedOrder,
106           CriteriaResolver[] memory criteriaResolvers,
107           bool revertOnInvalid,
108           bytes32[] memory priorOrderHashes
109       )
110           internal
111           returns (
112               bytes32 orderHash,
113               uint256 newNumerator,
114:              uint256 newDenominator

/// @audit newDenominator
104       function _validateOrderAndUpdateStatus(
105           AdvancedOrder memory advancedOrder,
106           CriteriaResolver[] memory criteriaResolvers,
107           bool revertOnInvalid,
108           bytes32[] memory priorOrderHashes
109       )
110           internal
111           returns (
112               bytes32 orderHash,
113               uint256 newNumerator,
114:              uint256 newDenominator

/// @audit isValidated
418       function _getOrderStatus(bytes32 orderHash)
419           internal
420           view
421           returns (
422               bool isValidated,
423               bool isCancelled,
424               uint256 totalFilled,
425:              uint256 totalSize

/// @audit isCancelled
418       function _getOrderStatus(bytes32 orderHash)
419           internal
420           view
421           returns (
422               bool isValidated,
423               bool isCancelled,
424               uint256 totalFilled,
425:              uint256 totalSize

/// @audit totalFilled
418       function _getOrderStatus(bytes32 orderHash)
419           internal
420           view
421           returns (
422               bool isValidated,
423               bool isCancelled,
424               uint256 totalFilled,
425:              uint256 totalSize

/// @audit totalSize
418       function _getOrderStatus(bytes32 orderHash)
419           internal
420           view
421           returns (
422               bool isValidated,
423               bool isCancelled,
424               uint256 totalFilled,
425:              uint256 totalSize

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

File: contracts/lib/Consideration.sol

/// @audit availableOrders
215       function fulfillAvailableOrders(
216           Order[] calldata orders,
217           FulfillmentComponent[][] calldata offerFulfillments,
218           FulfillmentComponent[][] calldata considerationFulfillments,
219           bytes32 fulfillerConduitKey,
220           uint256 maximumFulfilled
221       )
222           external
223           payable
224           override
225:          returns (bool[] memory availableOrders, Execution[] memory executions)

/// @audit executions
215       function fulfillAvailableOrders(
216           Order[] calldata orders,
217           FulfillmentComponent[][] calldata offerFulfillments,
218           FulfillmentComponent[][] calldata considerationFulfillments,
219           bytes32 fulfillerConduitKey,
220           uint256 maximumFulfilled
221       )
222           external
223           payable
224           override
225:          returns (bool[] memory availableOrders, Execution[] memory executions)

/// @audit availableOrders
300       function fulfillAvailableAdvancedOrders(
301           AdvancedOrder[] memory advancedOrders,
302           CriteriaResolver[] calldata criteriaResolvers,
303           FulfillmentComponent[][] calldata offerFulfillments,
304           FulfillmentComponent[][] calldata considerationFulfillments,
305           bytes32 fulfillerConduitKey,
306           uint256 maximumFulfilled
307       )
308           external
309           payable
310           override
311:          returns (bool[] memory availableOrders, Execution[] memory executions)

/// @audit executions
300       function fulfillAvailableAdvancedOrders(
301           AdvancedOrder[] memory advancedOrders,
302           CriteriaResolver[] calldata criteriaResolvers,
303           FulfillmentComponent[][] calldata offerFulfillments,
304           FulfillmentComponent[][] calldata considerationFulfillments,
305           bytes32 fulfillerConduitKey,
306           uint256 maximumFulfilled
307       )
308           external
309           payable
310           override
311:          returns (bool[] memory availableOrders, Execution[] memory executions)

/// @audit executions
349       function matchOrders(
350           Order[] calldata orders,
351           Fulfillment[] calldata fulfillments
352:      ) external payable override returns (Execution[] memory executions) {

/// @audit executions
398       function matchAdvancedOrders(
399           AdvancedOrder[] memory advancedOrders,
400           CriteriaResolver[] calldata criteriaResolvers,
401           Fulfillment[] calldata fulfillments
402:      ) external payable override returns (Execution[] memory executions) {

/// @audit isValidated
517       function getOrderStatus(bytes32 orderHash)
518           external
519           view
520           override
521           returns (
522               bool isValidated,
523               bool isCancelled,
524               uint256 totalFilled,
525:              uint256 totalSize

/// @audit isCancelled
517       function getOrderStatus(bytes32 orderHash)
518           external
519           view
520           override
521           returns (
522               bool isValidated,
523               bool isCancelled,
524               uint256 totalFilled,
525:              uint256 totalSize

/// @audit totalFilled
517       function getOrderStatus(bytes32 orderHash)
518           external
519           view
520           override
521           returns (
522               bool isValidated,
523               bool isCancelled,
524               uint256 totalFilled,
525:              uint256 totalSize

/// @audit totalSize
517       function getOrderStatus(bytes32 orderHash)
518           external
519           view
520           override
521           returns (
522               bool isValidated,
523               bool isCancelled,
524               uint256 totalFilled,
525:              uint256 totalSize

/// @audit version
556       function information()
557           external
558           view
559           override
560           returns (
561               string memory version,
562               bytes32 domainSeparator,
563:              address conduitController

/// @audit domainSeparator
556       function information()
557           external
558           view
559           override
560           returns (
561               string memory version,
562               bytes32 domainSeparator,
563:              address conduitController

/// @audit conduitController
556       function information()
557           external
558           view
559           override
560           returns (
561               string memory version,
562               bytes32 domainSeparator,
563:              address conduitController

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

File: reference/conduit/ReferenceConduit.sol

/// @audit magicValue
36        function execute(ConduitTransfer[] calldata transfers)
37            external
38            override
39:           returns (bytes4 magicValue)

/// @audit magicValue
59        function executeBatch1155(
60            ConduitBatch1155Transfer[] calldata batchTransfers
61:       ) external override returns (bytes4 magicValue) {

/// @audit magicValue
80        function executeWithBatch1155(
81            ConduitTransfer[] calldata standardTransfers,
82            ConduitBatch1155Transfer[] calldata batchTransfers
83:       ) external override returns (bytes4 magicValue) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduit.sol#L36-L39

File: reference/lib/ReferenceOrderCombiner.sol

/// @audit executions
694       function _matchAdvancedOrders(
695           AdvancedOrder[] memory advancedOrders,
696           CriteriaResolver[] memory criteriaResolvers,
697           Fulfillment[] calldata fulfillments
698:      ) internal returns (Execution[] memory executions) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L694-L698

File: reference/lib/ReferenceFulfillmentApplier.sol

/// @audit invalidFulfillment
238       function _checkMatchingOffer(
239           OrderToExecute memory orderToExecute,
240           SpentItem memory offer,
241           Execution memory execution
242:      ) internal pure returns (bool invalidFulfillment) {

/// @audit invalidFulfillment
407       function _checkMatchingConsideration(
408           ReceivedItem memory consideration,
409           ReceivedItem memory receivedItem
410:      ) internal pure returns (bool invalidFulfillment) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L238-L242

File: reference/lib/ReferenceOrderValidator.sol

/// @audit newNumerator
109       function _validateOrderAndUpdateStatus(
110           AdvancedOrder memory advancedOrder,
111           CriteriaResolver[] memory criteriaResolvers,
112           bool revertOnInvalid,
113           bytes32[] memory priorOrderHashes
114       )
115           internal
116           returns (
117               bytes32 orderHash,
118               uint256 newNumerator,
119:              uint256 newDenominator

/// @audit newDenominator
109       function _validateOrderAndUpdateStatus(
110           AdvancedOrder memory advancedOrder,
111           CriteriaResolver[] memory criteriaResolvers,
112           bool revertOnInvalid,
113           bytes32[] memory priorOrderHashes
114       )
115           internal
116           returns (
117               bytes32 orderHash,
118               uint256 newNumerator,
119:              uint256 newDenominator

/// @audit isValidated
388       function _getOrderStatus(bytes32 orderHash)
389           internal
390           view
391           returns (
392               bool isValidated,
393               bool isCancelled,
394               uint256 totalFilled,
395:              uint256 totalSize

/// @audit isCancelled
388       function _getOrderStatus(bytes32 orderHash)
389           internal
390           view
391           returns (
392               bool isValidated,
393               bool isCancelled,
394               uint256 totalFilled,
395:              uint256 totalSize

/// @audit totalFilled
388       function _getOrderStatus(bytes32 orderHash)
389           internal
390           view
391           returns (
392               bool isValidated,
393               bool isCancelled,
394               uint256 totalFilled,
395:              uint256 totalSize

/// @audit totalSize
388       function _getOrderStatus(bytes32 orderHash)
389           internal
390           view
391           returns (
392               bool isValidated,
393               bool isCancelled,
394               uint256 totalFilled,
395:              uint256 totalSize

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderValidator.sol#L109-L119

File: reference/lib/ReferenceGettersAndDerivers.sol

/// @audit orderHash
91        function _deriveOrderHash(
92            OrderParameters memory orderParameters,
93            uint256 nonce
94:       ) internal view returns (bytes32 orderHash) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L91-L94

File: reference/ReferenceConsideration.sol

/// @audit availableOrders
234       function fulfillAvailableOrders(
235           Order[] calldata orders,
236           FulfillmentComponent[][] calldata offerFulfillments,
237           FulfillmentComponent[][] calldata considerationFulfillments,
238           bytes32 fulfillerConduitKey,
239           uint256 maximumFulfilled
240       )
241           external
242           payable
243           override
244           notEntered
245           nonReentrant
246:          returns (bool[] memory availableOrders, Execution[] memory executions)

/// @audit executions
234       function fulfillAvailableOrders(
235           Order[] calldata orders,
236           FulfillmentComponent[][] calldata offerFulfillments,
237           FulfillmentComponent[][] calldata considerationFulfillments,
238           bytes32 fulfillerConduitKey,
239           uint256 maximumFulfilled
240       )
241           external
242           payable
243           override
244           notEntered
245           nonReentrant
246:          returns (bool[] memory availableOrders, Execution[] memory executions)

/// @audit availableOrders
332       function fulfillAvailableAdvancedOrders(
333           AdvancedOrder[] memory advancedOrders,
334           CriteriaResolver[] calldata criteriaResolvers,
335           FulfillmentComponent[][] calldata offerFulfillments,
336           FulfillmentComponent[][] calldata considerationFulfillments,
337           bytes32 fulfillerConduitKey,
338           uint256 maximumFulfilled
339       )
340           external
341           payable
342           override
343           notEntered
344           nonReentrant
345:          returns (bool[] memory availableOrders, Execution[] memory executions)

/// @audit executions
332       function fulfillAvailableAdvancedOrders(
333           AdvancedOrder[] memory advancedOrders,
334           CriteriaResolver[] calldata criteriaResolvers,
335           FulfillmentComponent[][] calldata offerFulfillments,
336           FulfillmentComponent[][] calldata considerationFulfillments,
337           bytes32 fulfillerConduitKey,
338           uint256 maximumFulfilled
339       )
340           external
341           payable
342           override
343           notEntered
344           nonReentrant
345:          returns (bool[] memory availableOrders, Execution[] memory executions)

/// @audit executions
390       function matchOrders(
391           Order[] calldata orders,
392           Fulfillment[] calldata fulfillments
393       )
394           external
395           payable
396           override
397           notEntered
398           nonReentrant
399:          returns (Execution[] memory executions)

/// @audit executions
446       function matchAdvancedOrders(
447           AdvancedOrder[] memory advancedOrders,
448           CriteriaResolver[] calldata criteriaResolvers,
449           Fulfillment[] calldata fulfillments
450       )
451           external
452           payable
453           override
454           notEntered
455           nonReentrant
456:          returns (Execution[] memory executions)

/// @audit isValidated
574       function getOrderStatus(bytes32 orderHash)
575           external
576           view
577           override
578           returns (
579               bool isValidated,
580               bool isCancelled,
581               uint256 totalFilled,
582:              uint256 totalSize

/// @audit isCancelled
574       function getOrderStatus(bytes32 orderHash)
575           external
576           view
577           override
578           returns (
579               bool isValidated,
580               bool isCancelled,
581               uint256 totalFilled,
582:              uint256 totalSize

/// @audit totalFilled
574       function getOrderStatus(bytes32 orderHash)
575           external
576           view
577           override
578           returns (
579               bool isValidated,
580               bool isCancelled,
581               uint256 totalFilled,
582:              uint256 totalSize

/// @audit totalSize
574       function getOrderStatus(bytes32 orderHash)
575           external
576           view
577           override
578           returns (
579               bool isValidated,
580               bool isCancelled,
581               uint256 totalFilled,
582:              uint256 totalSize

/// @audit version
613       function information()
614           external
615           view
616           override
617           returns (
618               string memory version,
619               bytes32 domainSeparator,
620:              address conduitController

/// @audit domainSeparator
613       function information()
614           external
615           view
616           override
617           returns (
618               string memory version,
619               bytes32 domainSeparator,
620:              address conduitController

/// @audit conduitController
613       function information()
614           external
615           view
616           override
617           returns (
618               string memory version,
619               bytes32 domainSeparator,
620:              address conduitController

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/ReferenceConsideration.sol#L234-L246

liveactionllama commented 2 years ago

Warden created this issue as a placeholder, because their submission was too large for the contest form. They then emailed their md file to our team on 06/03/2022 at 16:47 UTC. I've updated this issue with their md file content.

HardlyDifficult commented 2 years ago

Merging with https://github.com/code-423n4/2022-05-opensea-seaport-findings/issues/146