From ac1021ca3e6abac7cab31d7bf074c83ded849d53 Mon Sep 17 00:00:00 2001 From: Nando <24811313+fzavalia@users.noreply.github.com> Date: Wed, 10 Aug 2022 19:25:34 -0300 Subject: [PATCH 1/2] feat: Move signature verification --- contracts/Rentals.sol | 96 ++++++++++++++++++++++++------------------- test/Rentals.spec.ts | 31 ++++---------- 2 files changed, 63 insertions(+), 64 deletions(-) diff --git a/contracts/Rentals.sol b/contracts/Rentals.sol index 1d6fd58..8c6e951 100644 --- a/contracts/Rentals.sol +++ b/contracts/Rentals.sol @@ -192,27 +192,7 @@ contract Rentals is ) external nonReentrant { _verifyUnsafeTransfer(_listing.contractAddress, _listing.tokenId); - // Verify that the signer provided in the listing is the one that signed it. - bytes32 listingHash = _hashTypedDataV4( - keccak256( - abi.encode( - LISTING_TYPE_HASH, - _listing.signer, - _listing.contractAddress, - _listing.tokenId, - _listing.expiration, - keccak256(abi.encodePacked(_listing.nonces)), - keccak256(abi.encodePacked(_listing.pricePerDay)), - keccak256(abi.encodePacked(_listing.maxDays)), - keccak256(abi.encodePacked(_listing.minDays)), - _listing.target - ) - ) - ); - - address lessor = ECDSAUpgradeable.recover(listingHash, _listing.signature); - - require(lessor == _listing.signer, "Rentals#acceptListing: SIGNATURE_MISMATCH"); + address lessor = _listing.signer; // Verify that the caller and the signer are not the same address. address tenant = _msgSender(); @@ -249,6 +229,8 @@ contract Rentals is // Verify that the provided rental days is between min and max days range. require(_rentalDays >= minDays && _rentalDays <= maxDays, "Rentals#acceptListing: DAYS_NOT_IN_RANGE"); + _verifyListingSigner(_listing); + _rent( RentParams( lessor, @@ -423,27 +405,7 @@ contract Rentals is } function _acceptOffer(Offer memory _offer, address _lessor) private nonReentrant { - // Verify that the signer provided in the offer is the one that signed it. - bytes32 offerHash = _hashTypedDataV4( - keccak256( - abi.encode( - OFFER_TYPE_HASH, - _offer.signer, - _offer.contractAddress, - _offer.tokenId, - _offer.expiration, - keccak256(abi.encodePacked(_offer.nonces)), - _offer.pricePerDay, - _offer.rentalDays, - _offer.operator, - _offer.fingerprint - ) - ) - ); - - address tenant = ECDSAUpgradeable.recover(offerHash, _offer.signature); - - require(tenant == _offer.signer, "Rentals#acceptOffer: SIGNATURE_MISMATCH"); + address tenant = _offer.signer; require(_lessor != tenant, "Rentals#acceptOffer: CALLER_CANNOT_BE_SIGNER"); @@ -458,6 +420,8 @@ contract Rentals is // Verify that the rental days provided in the offer are valid. require(_offer.rentalDays > 0, "Rentals#acceptOffer: RENTAL_DAYS_IS_ZERO"); + _verifyOfferSigner(_offer); + _rent( RentParams( _lessor, @@ -473,6 +437,54 @@ contract Rentals is ); } + /// @dev Verify that the signer provided in the listing is the address that created the provided signature. + function _verifyListingSigner(Listing calldata _listing) private view { + bytes32 listingHash = _hashTypedDataV4( + keccak256( + abi.encode( + LISTING_TYPE_HASH, + _listing.signer, + _listing.contractAddress, + _listing.tokenId, + _listing.expiration, + keccak256(abi.encodePacked(_listing.nonces)), + keccak256(abi.encodePacked(_listing.pricePerDay)), + keccak256(abi.encodePacked(_listing.maxDays)), + keccak256(abi.encodePacked(_listing.minDays)), + _listing.target + ) + ) + ); + + address signer = ECDSAUpgradeable.recover(listingHash, _listing.signature); + + require(signer == _listing.signer, "Rentals#_verifyListingSigner: SIGNER_MISMATCH"); + } + + /// @dev Verify that the signer provided in the offer is the address that created the provided signature. + function _verifyOfferSigner(Offer memory _offer) private view { + bytes32 offerHash = _hashTypedDataV4( + keccak256( + abi.encode( + OFFER_TYPE_HASH, + _offer.signer, + _offer.contractAddress, + _offer.tokenId, + _offer.expiration, + keccak256(abi.encodePacked(_offer.nonces)), + _offer.pricePerDay, + _offer.rentalDays, + _offer.operator, + _offer.fingerprint + ) + ) + ); + + address signer = ECDSAUpgradeable.recover(offerHash, _offer.signature); + + require(signer == _offer.signer, "Rentals#_verifyOfferSigner: SIGNER_MISMATCH"); + } + function _rent(RentParams memory _rentParams) private { IERC721Rentable asset = IERC721Rentable(_rentParams.contractAddress); diff --git a/test/Rentals.spec.ts b/test/Rentals.spec.ts index 7dff267..be6d150 100644 --- a/test/Rentals.spec.ts +++ b/test/Rentals.spec.ts @@ -1073,7 +1073,7 @@ describe('Rentals', () => { acceptListingParams.rentalDays, acceptListingParams.fingerprint ) - ).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH') + ).to.be.revertedWith('Rentals#_verifyListingSigner: SIGNER_MISMATCH') }) it('should revert when pricePerDay maxDays and minDays length is 0', async () => { @@ -1518,9 +1518,8 @@ describe('Rentals', () => { it('should revert when the listing was signed with arrays in a different order than the ones provided', async () => { listingParams.pricePerDay = [ether('10'), ether('20'), ether('30')] - listingParams.maxDays = [10, 20, 30] - listingParams.minDays = [5, 15, 25] - listingParams.nonces = [1, 2, 3] + listingParams.maxDays = [98, 99, 100] + listingParams.minDays = [1, 2, 3] const signature = await getListingSignature(lessor, rentals, listingParams) @@ -1534,7 +1533,7 @@ describe('Rentals', () => { acceptListingParams.rentalDays, acceptListingParams.fingerprint ) - ).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH') + ).to.be.revertedWith('Rentals#_verifyListingSigner: SIGNER_MISMATCH') await expect( rentals @@ -1546,7 +1545,7 @@ describe('Rentals', () => { acceptListingParams.rentalDays, acceptListingParams.fingerprint ) - ).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH') + ).to.be.revertedWith('Rentals#_verifyListingSigner: SIGNER_MISMATCH') await expect( rentals @@ -1558,19 +1557,7 @@ describe('Rentals', () => { acceptListingParams.rentalDays, acceptListingParams.fingerprint ) - ).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH') - - await expect( - rentals - .connect(tenant) - .acceptListing( - { ...listingParams, nonces: listingParams.nonces.reverse() as [BigNumberish, BigNumberish, BigNumberish], signature }, - acceptListingParams.operator, - acceptListingParams.index, - acceptListingParams.rentalDays, - acceptListingParams.fingerprint - ) - ).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH') + ).to.be.revertedWith('Rentals#_verifyListingSigner: SIGNER_MISMATCH') }) }) @@ -1936,8 +1923,8 @@ describe('Rentals', () => { await expect( rentals .connect(lessor) - .acceptOffer({ ...offerParams, signature: await getOfferSignature(tenant, rentals, { ...offerParams, signer: lessor.address }) }) - ).to.be.revertedWith('Rentals#acceptOffer: SIGNATURE_MISMATCH') + .acceptOffer({ ...offerParams, signature: await getOfferSignature(tenant, rentals, { ...offerParams, signer: extra.address }) }) + ).to.be.revertedWith('Rentals#_verifyOfferSigner: SIGNER_MISMATCH') }) it('should revert when lessor is same as tenant', async () => { @@ -3212,7 +3199,7 @@ describe('Rentals', () => { await expect( land.connect(lessor)['safeTransferFrom(address,address,uint256,bytes)'](lessor.address, rentals.address, tokenId, bytes) - ).to.be.revertedWith('Rentals#acceptOffer: SIGNATURE_MISMATCH') + ).to.be.revertedWith('Rentals#_verifyOfferSigner: SIGNER_MISMATCH') }) it('should revert when lessor is same as tenant', async () => { From a3ff7fa6174fb4ee9b49ddeb563212675c3fd298 Mon Sep 17 00:00:00 2001 From: Nando <24811313+fzavalia@users.noreply.github.com> Date: Wed, 10 Aug 2022 19:26:53 -0300 Subject: [PATCH 2/2] fix: Test --- test/Rentals.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Rentals.spec.ts b/test/Rentals.spec.ts index be6d150..e31fca5 100644 --- a/test/Rentals.spec.ts +++ b/test/Rentals.spec.ts @@ -3193,7 +3193,7 @@ describe('Rentals', () => { }) it('should revert when the offer signer does not match the signer provided in params', async () => { - offerEncodeValue[signerIndex] = lessor.address + offerEncodeValue[signerIndex] = extra.address const bytes = ethers.utils.defaultAbiCoder.encode([offerEncodeType], [offerEncodeValue])