Add Foundry test for failures when spending pre-approved items from null address with invalid signature

test/foundry/FulfillOrderTest.t.sol

@@ -107,6 +107,45 @@ contract FulfillOrderTest is BaseOrderTest {
         _;
     }
 
+    function testNullAddressSpendReverts() public {
+        // mint token to null address
+        preapproved721.mint(address(0), 1);
+        // mint erc token to test address
+        token1.mint(address(this), 1);
+        // offer burnt erc721
+        addErc721OfferItem(address(preapproved721), 1);
+        // consider erc20 to null address
+        addErc20ConsiderationItem(payable(0), 1);
+        // configure baseOrderParameters with null address as offerer
+        configureOrderParameters(address(0));
+        test(
+            this.nullAddressSpendReverts,
+            Context(referenceConsideration, empty, 0, 0, 0)
+        );
+        test(
+            this.nullAddressSpendReverts,
+            Context(consideration, empty, 0, 0, 0)
+        );
+    }
+
+    function nullAddressSpendReverts(Context memory context)
+        external
+        stateless
+    {
+        // create a bad signature
+        bytes memory signature = abi.encodePacked(
+            bytes32(0),
+            bytes32(0),
+            bytes1(uint8(27))
+        );
+        // test that signature is recognized as invalid even though signer recovered is null address
+        vm.expectRevert(abi.encodeWithSignature("InvalidSigner()"));
+        context.consideration.fulfillOrder(
+            Order(baseOrderParameters, signature),
+            bytes32(0)
+        );
+    }
+
     function testFulfillAscendingDescendingOffer(FuzzInputsCommon memory inputs)
         public
         validateInputs(inputs)

test/foundry/token/ERC721.sol

@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: AGPL-3.0-only
+pragma solidity >=0.8.0;
+
+/// @notice Modern, minimalist, and gas efficient ERC-721 implementation.
+/// @author Solmate (https://github.com/Rari-Capital/solmate/blob/main/src/tokens/ERC721.sol)
+/// @notice modified for testing purposes
+abstract contract ERC721 {
+    /*//////////////////////////////////////////////////////////////
+                                 EVENTS
+    //////////////////////////////////////////////////////////////*/
+
+    event Transfer(
+        address indexed from,
+        address indexed to,
+        uint256 indexed id
+    );
+
+    event Approval(
+        address indexed owner,
+        address indexed spender,
+        uint256 indexed id
+    );
+
+    event ApprovalForAll(
+        address indexed owner,
+        address indexed operator,
+        bool approved
+    );
+
+    /*//////////////////////////////////////////////////////////////
+                         METADATA STORAGE/LOGIC
+    //////////////////////////////////////////////////////////////*/
+
+    string public name;
+
+    string public symbol;
+
+    function tokenURI(uint256 id) public view virtual returns (string memory);
+
+    /*//////////////////////////////////////////////////////////////
+                      ERC721 BALANCE/OWNER STORAGE
+    //////////////////////////////////////////////////////////////*/
+
+    mapping(uint256 => address) internal _ownerOf;
+
+    mapping(address => uint256) internal _balanceOf;
+
+    function ownerOf(uint256 id) public view virtual returns (address owner) {
+        require((owner = _ownerOf[id]) != address(0), "NOT_MINTED");
+    }
+
+    function balanceOf(address owner) public view virtual returns (uint256) {
+        require(owner != address(0), "ZERO_ADDRESS");
+
+        return _balanceOf[owner];
+    }
+
+    /*//////////////////////////////////////////////////////////////
+                         ERC721 APPROVAL STORAGE
+    //////////////////////////////////////////////////////////////*/
+
+    mapping(uint256 => address) public getApproved;
+
+    mapping(address => mapping(address => bool)) internal _isApprovedForAll;
+
+    /*//////////////////////////////////////////////////////////////
+                               CONSTRUCTOR
+    //////////////////////////////////////////////////////////////*/
+
+    constructor(string memory _name, string memory _symbol) {
+        name = _name;
+        symbol = _symbol;
+    }
+
+    /*//////////////////////////////////////////////////////////////
+                              ERC721 LOGIC
+    //////////////////////////////////////////////////////////////*/
+
+    function approve(address spender, uint256 id) public virtual {
+        address owner = _ownerOf[id];
+
+        require(
+            msg.sender == owner || isApprovedForAll(owner, msg.sender),
+            "NOT_AUTHORIZED"
+        );
+
+        getApproved[id] = spender;
+
+        emit Approval(owner, spender, id);
+    }
+
+    function setApprovalForAll(address operator, bool approved) public virtual {
+        _isApprovedForAll[msg.sender][operator] = approved;
+
+        emit ApprovalForAll(msg.sender, operator, approved);
+    }
+
+    function isApprovedForAll(address owner, address spender)
+        public
+        view
+        virtual
+        returns (bool)
+    {
+        return _isApprovedForAll[owner][spender];
+    }
+
+    function transferFrom(
+        address from,
+        address to,
+        uint256 id
+    ) public virtual {
+        require(from == _ownerOf[id], "WRONG_FROM");
+
+        require(
+            msg.sender == from ||
+                isApprovedForAll(from, msg.sender) ||
+                msg.sender == getApproved[id],
+            "NOT_AUTHORIZED"
+        );
+
+        // Underflow of the sender's balance is impossible because we check for
+        // ownership above and the recipient's balance can't realistically overflow.
+        unchecked {
+            _balanceOf[from]--;
+
+            _balanceOf[to]++;
+        }
+
+        _ownerOf[id] = to;
+
+        delete getApproved[id];
+
+        emit Transfer(from, to, id);
+    }
+
+    function safeTransferFrom(
+        address from,
+        address to,
+        uint256 id
+    ) public virtual {
+        transferFrom(from, to, id);
+
+        require(
+            to.code.length == 0 ||
+                ERC721TokenReceiver(to).onERC721Received(
+                    msg.sender,
+                    from,
+                    id,
+                    ""
+                ) ==
+                ERC721TokenReceiver.onERC721Received.selector,
+            "UNSAFE_RECIPIENT"
+        );
+    }
+
+    function safeTransferFrom(
+        address from,
+        address to,
+        uint256 id,
+        bytes calldata data
+    ) public virtual {
+        transferFrom(from, to, id);
+
+        require(
+            to.code.length == 0 ||
+                ERC721TokenReceiver(to).onERC721Received(
+                    msg.sender,
+                    from,
+                    id,
+                    data
+                ) ==
+                ERC721TokenReceiver.onERC721Received.selector,
+            "UNSAFE_RECIPIENT"
+        );
+    }
+
+    /*//////////////////////////////////////////////////////////////
+                              ERC165 LOGIC
+    //////////////////////////////////////////////////////////////*/
+
+    function supportsInterface(bytes4 interfaceId)
+        public
+        view
+        virtual
+        returns (bool)
+    {
+        return
+            interfaceId == 0x01ffc9a7 || // ERC165 Interface ID for ERC165
+            interfaceId == 0x80ac58cd || // ERC165 Interface ID for ERC721
+            interfaceId == 0x5b5e139f; // ERC165 Interface ID for ERC721Metadata
+    }
+
+    /*//////////////////////////////////////////////////////////////
+                        INTERNAL MINT/BURN LOGIC
+    //////////////////////////////////////////////////////////////*/
+
+    function _mint(address to, uint256 id) internal virtual {
+        require(_ownerOf[id] == address(0), "ALREADY_MINTED");
+
+        // Counter overflow is incredibly unrealistic.
+        unchecked {
+            _balanceOf[to]++;
+        }
+
+        _ownerOf[id] = to;
+
+        emit Transfer(address(0), to, id);
+    }
+
+    function _burn(uint256 id) internal virtual {
+        address owner = _ownerOf[id];
+
+        require(owner != address(0), "NOT_MINTED");
+
+        // Ownership check above ensures no underflow.
+        unchecked {
+            _balanceOf[owner]--;
+        }
+
+        delete _ownerOf[id];
+
+        delete getApproved[id];
+
+        emit Transfer(owner, address(0), id);
+    }
+
+    /*//////////////////////////////////////////////////////////////
+                        INTERNAL SAFE MINT LOGIC
+    //////////////////////////////////////////////////////////////*/
+
+    function _safeMint(address to, uint256 id) internal virtual {
+        _mint(to, id);
+
+        require(
+            to.code.length == 0 ||
+                ERC721TokenReceiver(to).onERC721Received(
+                    msg.sender,
+                    address(0),
+                    id,
+                    ""
+                ) ==
+                ERC721TokenReceiver.onERC721Received.selector,
+            "UNSAFE_RECIPIENT"
+        );
+    }
+
+    function _safeMint(
+        address to,
+        uint256 id,
+        bytes memory data
+    ) internal virtual {
+        _mint(to, id);
+
+        require(
+            to.code.length == 0 ||
+                ERC721TokenReceiver(to).onERC721Received(
+                    msg.sender,
+                    address(0),
+                    id,
+                    data
+                ) ==
+                ERC721TokenReceiver.onERC721Received.selector,
+            "UNSAFE_RECIPIENT"
+        );
+    }
+}
+
+/// @notice A generic interface for a contract which properly accepts ERC721 tokens.
+/// @author Solmate (https://github.com/Rari-Capital/solmate/blob/main/src/tokens/ERC721.sol)
+abstract contract ERC721TokenReceiver {
+    function onERC721Received(
+        address,
+        address,
+        uint256,
+        bytes calldata
+    ) external virtual returns (bytes4) {
+        return ERC721TokenReceiver.onERC721Received.selector;
+    }
+}

test/foundry/utils/BaseOrderTest.sol

@@ -219,7 +219,7 @@ contract BaseOrderTest is OfferConsiderationItemAdder, AmountDeriver {
         uint256 originalItemsLength,
         uint256 amtToSubtractFromItemsLength
     ) internal {
-        assertTrue(_validateOrder(order, consideration));
+        assertTrue(_validateOrder(order, _consideration));
 
         bool overwriteItemsLength = amtToSubtractFromItemsLength > 0;
         if (overwriteItemsLength) {
@@ -251,7 +251,7 @@ contract BaseOrderTest is OfferConsiderationItemAdder, AmountDeriver {
         );
 
         bool success = _callConsiderationFulfillOrderWithCalldata(
-            address(consideration),
+            address(_consideration),
             fulfillOrderCalldata
         );
 

test/foundry/utils/OfferConsiderationItemAdder.sol

@@ -52,8 +52,35 @@ contract OfferConsiderationItemAdder is TestTokenMinter {
         addOfferItem(itemType, identifier, amt, amt);
     }
 
-    function addErc721OfferItem(uint256 tokenId) internal {
-        addOfferItem(ItemType.ERC721, address(test721_1), tokenId, 1, 1);
+    function addErc721OfferItem(uint256 identifier) internal {
+        addErc721OfferItem(address(test721_1), identifier);
+    }
+
+    function addErc721OfferItem(address token, uint256 identifier) internal {
+        addErc721OfferItem(token, identifier, 1, 1);
+    }
+
+    function addErc721OfferItem(
+        address token,
+        uint256 identifier,
+        uint256 amount
+    ) internal {
+        addErc721OfferItem(token, identifier, amount, amount);
+    }
+
+    function addErc721OfferItem(
+        address token,
+        uint256 identifier,
+        uint256 startAmount,
+        uint256 endAmount
+    ) internal {
+        addOfferItem(
+            ItemType.ERC721,
+            token,
+            identifier,
+            startAmount,
+            endAmount
+        );
     }
 
     function addErc1155OfferItem(uint256 tokenId, uint256 amount) internal {