Merge pull request #378 from unstoppabledomains/kiryl/reg-1600-update…

…-seaportproxybuyer-to-prevent-seaport-order-execution

[REG-1600] - Update SeaportProxyBuyer to prevent Seaport order execution outside of SeaportProxyBuyer call

CHANGELOG.md

@@ -1,4 +1,5 @@
 ## v0.9.41
+- Add revert if Seaport order is executed outside `SeaportProxyBuyer` contract
 - Added a script to batch mint TLDs proposal
 
 ## v0.9.40

contracts/marketplace/ISeaportProxyBuyer.sol

@@ -8,6 +8,7 @@ import {AdvancedOrder, CriteriaResolver, ZoneParameters} from 'seaport-types/src
 error OrderIsNotFulfiled();
 error RecipientIsZeroAddress();
 error InvalidZone();
+error InvalidFulfiller();
 
 interface ISeaportProxyBuyer {
     /**

contracts/marketplace/SeaportProxyBuyer.sol

@@ -11,7 +11,7 @@ import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
 import {ERC2771RegistryContext} from '../metatx/ERC2771RegistryContext.sol';
 import {Forwarder} from '../metatx/Forwarder.sol';
 import {MinterRole} from '../roles/MinterRole.sol';
-import {ISeaportProxyBuyer, OrderIsNotFulfiled, RecipientIsZeroAddress, InvalidZone} from './ISeaportProxyBuyer.sol';
+import {ISeaportProxyBuyer, OrderIsNotFulfiled, RecipientIsZeroAddress, InvalidZone, InvalidFulfiller} from './ISeaportProxyBuyer.sol';
 import {ConsiderationInterface} from 'seaport-types/src/interfaces/ConsiderationInterface.sol';
 import {AdvancedOrder, CriteriaResolver, OrderComponents, OrderParameters, ZoneParameters} from 'seaport-types/src/lib/ConsiderationStructs.sol';
 
@@ -26,7 +26,7 @@ contract SeaportProxyBuyer is
     ISeaportProxyBuyer
 {
     string public constant NAME = 'Seaport Proxy Buyer';
-    string public constant VERSION = '0.1.1';
+    string public constant VERSION = '0.1.2';
 
     ConsiderationInterface private _seaport;
 
@@ -87,11 +87,11 @@ contract SeaportProxyBuyer is
     }
 
     function authorizeOrder(ZoneParameters calldata) external view override whenNotPaused returns (bytes4 authorizedOrderMagicValue) {
-        authorizedOrderMagicValue = ISeaportProxyBuyer.authorizeOrder.selector;
+        revert InvalidFulfiller();
     }
 
     function validateOrder(ZoneParameters calldata) external view override whenNotPaused returns (bytes4 validOrderMagicValue) {
-        validOrderMagicValue = ISeaportProxyBuyer.validateOrder.selector;
+        revert InvalidFulfiller();
     }
 
     function approve(address token) external onlyOwner nonReentrant {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "uns",
-  "version": "0.9.40",
+  "version": "0.9.41",
   "description": "UNS contracts and tools",
   "repository": "https://github.com/unstoppabledomains/uns.git",
   "main": "./dist/index.js",

test/marketplace/SeaportProxyBuyer.test.ts

@@ -739,6 +739,71 @@ describe('SeaportProxyBuyer', async () => {
     });
   });
 
+  describe('Zone security', () => {
+    it('should not allow to fulfill order outside of the zone', async () => {
+      const priceToSell = BigInt(ethers.parseUnits('100', 6));
+      const recipientFeesBasisPoints = BigInt(50);
+      const { fulfillOrderData, numerator, denominator } = await createSellOrder(
+        priceToSell,
+        recipientFeesBasisPoints,
+        proxyBuyerAddress,
+      );
+
+      await expect(
+        seaportContract
+          .connect(coinbase)
+          .fulfillAdvancedOrder(
+            { ...fulfillOrderData, numerator, denominator, extraData: '0x' },
+            [],
+            ethers.ZeroHash,
+            buyer.address,
+          ),
+      ).to.be.revertedWithCustomError(seaportProxyBuyer, 'InvalidFulfiller');
+    });
+
+    it('should not allow to match orders outside of the zone', async () => {
+      await usdcMock.connect(buyer).approve(await seaportContract.getAddress(), ethers.MaxUint256);
+      await unsRegistry.connect(seller).setApprovalForAll(await seaportContract.getAddress(), true);
+
+      const price = BigInt(ethers.parseUnits('100', 6));
+      const recipientFeesBasisPoints = BigInt(0);
+
+      await usdcMock.mint(buyer.address, price);
+
+      const sellOrder = await createSellOrder(price, recipientFeesBasisPoints, proxyBuyerAddress);
+      const buyOrder = await createBuyOrder(price, recipientFeesBasisPoints);
+
+      const orders: AdvancedOrderStruct[] = [
+        {
+          ...sellOrder.fulfillOrderData,
+          numerator: sellOrder.numerator,
+          denominator: sellOrder.denominator,
+          extraData: '0x',
+        },
+        {
+          ...buyOrder.fulfillOrderData,
+          numerator: buyOrder.numerator,
+          denominator: buyOrder.denominator,
+          extraData: '0x',
+        },
+      ];
+      const fulfillments: MatchOrdersFulfillment[] = [
+        {
+          offerComponents: [{ orderIndex: 0, itemIndex: 0 }],
+          considerationComponents: [{ orderIndex: 1, itemIndex: 0 }],
+        }, // NFT from Order 1 to Buyer in Order 2
+        {
+          offerComponents: [{ orderIndex: 1, itemIndex: 0 }],
+          considerationComponents: [{ orderIndex: 0, itemIndex: 0 }],
+        }, // USDC from Order 2 to Seller in Order 1
+      ];
+
+      await expect(
+        seaportContract.connect(reader).matchAdvancedOrders(orders, [], fulfillments, proxyBuyerAddress),
+      ).to.be.revertedWithCustomError(seaportProxyBuyer, 'InvalidFulfiller');
+    });
+  });
+
   describe('Match orders', async () => {
     it('should match 2 fullfiling orders from a random caller using ProxyBuyer as a zone', async () => {
       await usdcMock.connect(buyer).approve(await seaportContract.getAddress(), ethers.MaxUint256);
@@ -796,7 +861,7 @@ describe('SeaportProxyBuyer', async () => {
       expect(domainOwner).to.be.eq(buyer.address);
       expect(initialSeaportProxyBuyerBalance).to.be.eq(seaportProxyBuyerBalance);
       expect(readerBalance).to.be.eq(0);
-    });
+    }).skip();
 
     it('should match 2 fullfiling orders from a random caller using ProxyBuyer as a zone with a price leftover', async () => {
       await usdcMock.connect(buyer).approve(await seaportContract.getAddress(), ethers.MaxUint256);
@@ -863,7 +928,7 @@ describe('SeaportProxyBuyer', async () => {
       expect(domainOwner).to.be.eq(buyer.address);
       expect(initialSeaportProxyBuyerBalance).to.be.eq(seaportProxyBuyerBalance - (priceToBuy - priceToSell));
       expect(readerBalance).to.be.eq(0);
-    });
+    }).skip();
 
     it('should not allow to match orders with paused SeaportProxyBuyer', async () => {
       await usdcMock.connect(buyer).approve(await seaportContract.getAddress(), ethers.MaxUint256);
@@ -908,6 +973,6 @@ describe('SeaportProxyBuyer', async () => {
           .connect(reader)
           .matchAdvancedOrders(orders, [], fulfillments, await seaportProxyBuyer.getAddress()),
       ).to.be.revertedWith('Pausable: paused');
-    });
+    }).skip();
   });
 });