From d56c8331854102f9f77f3c57a86b151841cf5e10 Mon Sep 17 00:00:00 2001 From: Mark Toda Date: Fri, 5 Apr 2024 15:19:57 -0400 Subject: [PATCH] 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 --- ...V2DutchOrder-BaseExecuteSingleWithFee.snap | 2 +- .../Base-V2DutchOrder-ExclusiveFiller.snap | 2 +- .../Base-V2DutchOrder-ExecuteBatch.snap | 2 +- ...utchOrder-ExecuteBatchMultipleOutputs.snap | 2 +- ...teBatchMultipleOutputsDifferentTokens.snap | 2 +- ...V2DutchOrder-ExecuteBatchNativeOutput.snap | 2 +- .../Base-V2DutchOrder-ExecuteSingle.snap | 2 +- ...2DutchOrder-ExecuteSingleNativeOutput.snap | 2 +- ...-V2DutchOrder-ExecuteSingleValidation.snap | 2 +- .../Base-V2DutchOrder-InputOverride.snap | 2 +- .../Base-V2DutchOrder-OutputOverride.snap | 2 +- src/reactors/V2DutchOrderReactor.sol | 2 +- test/reactors/V2DutchOrderReactor.t.sol | 28 ++++++++++++++++++- 13 files changed, 39 insertions(+), 13 deletions(-) diff --git a/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap b/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap index 601dff7d..a583ac2c 100644 --- a/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap +++ b/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap @@ -1 +1 @@ -188641 \ No newline at end of file +188652 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExclusiveFiller.snap b/.forge-snapshots/Base-V2DutchOrder-ExclusiveFiller.snap index 5677f710..df812dd9 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExclusiveFiller.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExclusiveFiller.snap @@ -1 +1 @@ -158755 \ No newline at end of file +158766 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap index 6c6670d1..e8e31f23 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap @@ -1 +1 @@ -210706 \ No newline at end of file +210728 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap index dd014aba..2744d377 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap @@ -1 +1 @@ -220975 \ No newline at end of file +220997 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap index 3e76aa79..c0830615 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap @@ -1 +1 @@ -275146 \ No newline at end of file +275168 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap index f3b5e6a6..6d59fdab 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap @@ -1 +1 @@ -204232 \ No newline at end of file +204254 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap index 5d46966b..11cee36c 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap @@ -1 +1 @@ -155058 \ No newline at end of file +155069 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap index 0e39af06..7b7b7e20 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap @@ -1 +1 @@ -140620 \ No newline at end of file +140631 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap index 8e5ca1e0..00609fa9 100644 --- a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap +++ b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap @@ -1 +1 @@ -164369 \ No newline at end of file +164380 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-InputOverride.snap b/.forge-snapshots/Base-V2DutchOrder-InputOverride.snap index 5fa28dab..a8aa2096 100644 --- a/.forge-snapshots/Base-V2DutchOrder-InputOverride.snap +++ b/.forge-snapshots/Base-V2DutchOrder-InputOverride.snap @@ -1 +1 @@ -159044 \ No newline at end of file +159055 \ No newline at end of file diff --git a/.forge-snapshots/Base-V2DutchOrder-OutputOverride.snap b/.forge-snapshots/Base-V2DutchOrder-OutputOverride.snap index 46886e0b..bb228056 100644 --- a/.forge-snapshots/Base-V2DutchOrder-OutputOverride.snap +++ b/.forge-snapshots/Base-V2DutchOrder-OutputOverride.snap @@ -1 +1 @@ -158782 \ No newline at end of file +158793 \ No newline at end of file diff --git a/src/reactors/V2DutchOrderReactor.sol b/src/reactors/V2DutchOrderReactor.sol index fec9c873..f7ca769d 100644 --- 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 index 7c47847b..63f78a1d 100644 --- 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,