feat: handle invalid signature

This commit handles the case where cosignature is invalid, reverting. It also removes the special casing for address(0) cosigner specified by the user, as that would enable any filler to arbitrarily set the auction timings Issue: Cantina #23
Uniswap · Apr 15, 2024 · d56c833 · d56c833
1 parent 88a7f94
commit d56c833
Show file tree

Hide file tree

Showing 13 changed files with 39 additions and 13 deletions.
diff --git a/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap b/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap
@@ -1 +1 @@
-188641
+188652
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExclusiveFiller.snap b/.forge-snapshots/Base-V2DutchOrder-ExclusiveFiller.snap
@@ -1 +1 @@
-158755
+158766
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap
@@ -1 +1 @@
-210706
+210728
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap
@@ -1 +1 @@
-220975
+220997
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap
@@ -1 +1 @@
-275146
+275168
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap
@@ -1 +1 @@
-204232
+204254
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap
@@ -1 +1 @@
-155058
+155069
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap
@@ -1 +1 @@
-140620
+140631
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap
@@ -1 +1 @@
-164369
+164380
diff --git a/.forge-snapshots/Base-V2DutchOrder-InputOverride.snap b/.forge-snapshots/Base-V2DutchOrder-InputOverride.snap
@@ -1 +1 @@
-159044
+159055
diff --git a/.forge-snapshots/Base-V2DutchOrder-OutputOverride.snap b/.forge-snapshots/Base-V2DutchOrder-OutputOverride.snap
@@ -1 +1 @@
-158782
+158793
diff --git a/src/reactors/V2DutchOrderReactor.sol b/src/reactors/V2DutchOrderReactor.sol
@@ -124,7 +124,7 @@ contract V2DutchOrderReactor is BaseReactor {
         uint8 v = uint8(order.cosignature[64]);
         // cosigner signs over (orderHash || cosignerData)
         address signer = ecrecover(keccak256(abi.encodePacked(orderHash, abi.encode(order.cosignerData))), v, r, s);
-        if (order.cosigner != signer && signer != address(0)) {
+        if (order.cosigner != signer || signer == address(0)) {
             revert InvalidCosignature();
         }
 

diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol
@@ -121,7 +121,7 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact
         return (SignedOrder(abi.encode(order), signOrder(swapperPrivateKey, address(permit2), order)), orderHash);
     }
 
-    function testInvalidCosignature() public {
+    function testWrongCosigner() public {
         address wrongCosigner = makeAddr("wrongCosigner");
         CosignerData memory cosignerData = CosignerData({
             decayStartTime: block.timestamp,
@@ -147,6 +147,32 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact
         fillContract.execute(signedOrder);
     }
 
+    function testInvalidCosignature() public {
+        address wrongCosigner = makeAddr("wrongCosigner");
+        CosignerData memory cosignerData = CosignerData({
+            decayStartTime: block.timestamp,
+            decayEndTime: block.timestamp + 100,
+            exclusiveFiller: address(0),
+            exclusivityOverrideBps: 0,
+            inputAmount: 1 ether,
+            outputAmounts: ArrayBuilder.fill(1, 1 ether)
+        });
+
+        V2DutchOrder memory order = V2DutchOrder({
+            info: OrderInfoBuilder.init(address(reactor)).withSwapper(swapper),
+            cosigner: wrongCosigner,
+            baseInput: DutchInput(tokenIn, 1 ether, 1 ether),
+            baseOutputs: OutputsBuilder.singleDutch(address(tokenOut), 1 ether, 1 ether, swapper),
+            cosignerData: cosignerData,
+            cosignature: bytes("")
+        });
+        order.cosignature = bytes.concat(keccak256("invalidSignature"), keccak256("invalidSignature"), hex"33");
+        SignedOrder memory signedOrder =
+            SignedOrder(abi.encode(order), signOrder(swapperPrivateKey, address(permit2), order));
+        vm.expectRevert(V2DutchOrderReactor.InvalidCosignature.selector);
+        fillContract.execute(signedOrder);
+    }
+
     function testInputOverrideWorse() public {
         CosignerData memory cosignerData = CosignerData({
             decayStartTime: block.timestamp,