From 2129296f72bf67ecfdbcd6e06c5959d8a2fb8d7b Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 31 Oct 2023 15:56:03 +0000 Subject: [PATCH] Update MulticallerWithSigner with defensive signing --- .gas-snapshot | 34 +++++++++++++++--------------- src/MulticallerWithSigner.sol | 39 +++++++++++++++++++---------------- test/Multicaller.t.sol | 13 ++++++++---- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index f8ad8e0..e9eb8e2 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,31 +1,31 @@ -MulticallerTest:testMultiCallerWithSignerIncrementNonceSalt(uint256) (runs: 256, μ: 80066, ~: 81872) -MulticallerTest:testMultiCallerWithSignerIncrementNonceSaltWithERC1271(uint256) (runs: 256, μ: 580304, ~: 580357) -MulticallerTest:testMulticallerCdFallback(string) (runs: 256, μ: 306501, ~: 301746) +MulticallerTest:testMultiCallerWithSignerIncrementNonceSalt(uint256) (runs: 256, μ: 81621, ~: 80505) +MulticallerTest:testMultiCallerWithSignerIncrementNonceSaltWithERC1271(uint256) (runs: 256, μ: 580741, ~: 580643) +MulticallerTest:testMulticallerCdFallback(string) (runs: 256, μ: 295907, ~: 281868) MulticallerTest:testMulticallerForwardsMessageValue() (gas: 214013) MulticallerTest:testMulticallerGetNames() (gas: 147637) MulticallerTest:testMulticallerReentrancyGuard() (gas: 19980) -MulticallerTest:testMulticallerRefund(uint256) (runs: 256, μ: 169898, ~: 172026) +MulticallerTest:testMulticallerRefund(uint256) (runs: 256, μ: 169608, ~: 172026) MulticallerTest:testMulticallerReturnDataIsProperlyEncoded() (gas: 122173) -MulticallerTest:testMulticallerReturnDataIsProperlyEncoded(string,string,uint256) (runs: 256, μ: 520631, ~: 527233) +MulticallerTest:testMulticallerReturnDataIsProperlyEncoded(string,string,uint256) (runs: 256, μ: 474855, ~: 450231) MulticallerTest:testMulticallerReturnDataIsProperlyEncoded(uint256,uint256,uint256,uint256) (runs: 256, μ: 122252, ~: 122252) MulticallerTest:testMulticallerRevertWithCustomError() (gas: 35485) MulticallerTest:testMulticallerRevertWithMessage() (gas: 38244) -MulticallerTest:testMulticallerRevertWithMessage(string) (runs: 256, μ: 39122, ~: 39181) +MulticallerTest:testMulticallerRevertWithMessage(string) (runs: 256, μ: 38922, ~: 38644) MulticallerTest:testMulticallerRevertWithNothing() (gas: 35306) MulticallerTest:testMulticallerSenderDoesNotRevertWithoutMulticallerDeployed() (gas: 3423) MulticallerTest:testMulticallerTargetGetMulticallerSender() (gas: 27448) MulticallerTest:testMulticallerWithNoData() (gas: 16213) -MulticallerTest:testMulticallerWithSigner(uint256) (runs: 256, μ: 127763, ~: 118802) -MulticallerTest:testMulticallerWithSignerEIP712Domain() (gas: 12375) -MulticallerTest:testMulticallerWithSignerGetMulticallerSigner() (gas: 135610) -MulticallerTest:testMulticallerWithSignerInvalidateNonces(uint256) (runs: 256, μ: 79498, ~: 77716) -MulticallerTest:testMulticallerWithSignerInvalidateNoncesWithERC1271(uint256) (runs: 256, μ: 584890, ~: 584632) -MulticallerTest:testMulticallerWithSignerNonPayableFunctions() (gas: 48884) -MulticallerTest:testMulticallerWithSignerReentrancyGuard() (gas: 124037) -MulticallerTest:testMulticallerWithSignerRevert() (gas: 203818) -MulticallerTest:testMulticallerWithSignerWithERC1271(uint256) (runs: 256, μ: 613068, ~: 601565) -MulticallerTest:testMulticallerWithSignerWithNoData() (gas: 128593) +MulticallerTest:testMulticallerWithSigner(uint256) (runs: 256, μ: 127686, ~: 118866) +MulticallerTest:testMulticallerWithSignerEIP712Domain() (gas: 12295) +MulticallerTest:testMulticallerWithSignerGetMulticallerSigner() (gas: 135893) +MulticallerTest:testMulticallerWithSignerInvalidateNonces(uint256) (runs: 256, μ: 81253, ~: 80392) +MulticallerTest:testMulticallerWithSignerInvalidateNoncesWithERC1271(uint256) (runs: 256, μ: 584987, ~: 585039) +MulticallerTest:testMulticallerWithSignerNonPayableFunctions() (gas: 48775) +MulticallerTest:testMulticallerWithSignerReentrancyGuard() (gas: 124559) +MulticallerTest:testMulticallerWithSignerRevert() (gas: 204596) +MulticallerTest:testMulticallerWithSignerWithERC1271(uint256) (runs: 256, μ: 629700, ~: 621639) +MulticallerTest:testMulticallerWithSignerWithNoData() (gas: 129263) MulticallerTest:testNastyCalldataRevert() (gas: 3420) MulticallerTest:testOffsetTrick(uint256,uint256,uint256) (runs: 256, μ: 571, ~: 571) -MulticallerTest:test__codesize() (gas: 49524) +MulticallerTest:test__codesize() (gas: 53604) TestPlus:test__codesize() (gas: 1102) \ No newline at end of file diff --git a/src/MulticallerWithSigner.sol b/src/MulticallerWithSigner.sol index fc5ab8a..1bf8a24 100644 --- a/src/MulticallerWithSigner.sol +++ b/src/MulticallerWithSigner.sol @@ -48,26 +48,26 @@ contract MulticallerWithSigner { /** * @dev For EIP-712 signature digest calculation for the * `aggregateWithSigner` function. - * `keccak256("AggregateWithSigner(address[] targets,bytes[] data,uint256[] values,uint256 nonce,uint256 nonceSalt)")`. + * `keccak256("AggregateWithSigner(address signer,address[] targets,bytes[] data,uint256[] values,uint256 nonce,uint256 nonceSalt)")`. */ bytes32 private constant _AGGREGATE_WITH_SIGNER_TYPEHASH = - 0x7d4195b902a78aa23ae8c64d4cecdf8424f3171e7c7e34ed94e6fab3efd018ab; + 0xfb989fd34c8af81a76f18167f528fc7315f92cacc19a0e63215abd54633f8a28; /** * @dev For EIP-712 signature digest calculation for the * `invalidateNoncesForSigner` function. - * `keccak256("InvalidateNoncesForSigner(uint256[] nonces,uint256 nonceSalt)")`. + * `keccak256("InvalidateNoncesForSigner(address signer,uint256[] nonces,uint256 nonceSalt)")`. */ bytes32 private constant _INVALIDATE_NONCES_FOR_SIGNER_TYPEHASH = - 0xe75b4aefef1358e66ac7ed2f180022e0a7f661dcd2781630ce58e05bb8bdb1c1; + 0x12b047058eea3df4085cdc159a103d9c100c4e78cfb7029cc39d02cb8b9e48f5; /** * @dev For EIP-712 signature digest calculation for the * `incrementNonceSaltForSigner` function. - * `keccak256("IncrementNonceSaltForSigner(uint256 nonceSalt)")`. + * `keccak256("IncrementNonceSaltForSigner(address signer,uint256 nonceSalt)")`. */ bytes32 private constant _INCREMENT_NONCE_SALT_FOR_SIGNER_TYPEHASH = - 0x898da98c106c91ce6f05405740b0ed23b5c4dc847a0dd1996fb93189d8310bef; + 0xfa181078c7d1d4d369301511d3c5611e9367d0cebbf65eefdee9dfc75849c1d3; /** * @dev For EIP-712 signature digest calculation. @@ -194,12 +194,13 @@ contract MulticallerWithSigner { // Layout the fields of the struct hash. mstore(returndatasize(), _AGGREGATE_WITH_SIGNER_TYPEHASH) - mstore(0x20, targetsHash) - mstore(0x40, dataHash) - mstore(0x60, valuesHash) - mstore(0x80, nonce) - mstore(0xa0, sload(add(signer, address()))) // Store the nonce salt. - mstore(0x40, keccak256(returndatasize(), 0xc0)) // Compute and store the struct hash. + mstore(0x20, signer) + mstore(0x40, targetsHash) + mstore(0x60, dataHash) + mstore(0x80, valuesHash) + mstore(0xa0, nonce) + mstore(0xc0, sload(add(signer, address()))) // Store the nonce salt. + mstore(0x40, keccak256(returndatasize(), 0xe0)) // Compute and store the struct hash. // Layout the fields of the domain separator. mstore(0x60, _DOMAIN_TYPEHASH) mstore(0x80, _NAME_HASH) @@ -362,11 +363,12 @@ contract MulticallerWithSigner { let end := shl(5, nonces.length) // Layout the fields of the struct hash. mstore(returndatasize(), _INVALIDATE_NONCES_FOR_SIGNER_TYPEHASH) + mstore(0x20, signer) // Compute and store `keccak256(abi.encodePacked(nonces))`. - calldatacopy(0x20, nonces.offset, end) - mstore(0x20, keccak256(0x20, end)) - mstore(0x40, sload(add(signer, address()))) // Store the nonce salt. - mstore(0x40, keccak256(returndatasize(), 0x60)) // Compute and store the struct hash. + calldatacopy(0x40, nonces.offset, end) + mstore(0x40, keccak256(0x40, end)) + mstore(0x60, sload(add(signer, address()))) // Store the nonce salt. + mstore(0x40, keccak256(returndatasize(), 0x80)) // Compute and store the struct hash. // Layout the fields of the domain separator. mstore(0x60, _DOMAIN_TYPEHASH) mstore(0x80, _NAME_HASH) @@ -483,8 +485,9 @@ contract MulticallerWithSigner { let nonceSalt := sload(nonceSaltSlot) // Layout the fields of the struct hash. mstore(returndatasize(), _INCREMENT_NONCE_SALT_FOR_SIGNER_TYPEHASH) - mstore(0x20, nonceSalt) // Store the nonce salt. - mstore(0x40, keccak256(returndatasize(), 0x40)) // Compute and store the struct hash. + mstore(0x20, signer) + mstore(0x40, nonceSalt) // Store the nonce salt. + mstore(0x40, keccak256(returndatasize(), 0x60)) // Compute and store the struct hash. // Layout the fields of the domain separator. mstore(0x60, _DOMAIN_TYPEHASH) mstore(0x80, _NAME_HASH) diff --git a/test/Multicaller.t.sol b/test/Multicaller.t.sol index f35153a..ec2ac02 100644 --- a/test/Multicaller.t.sol +++ b/test/Multicaller.t.sol @@ -123,7 +123,7 @@ contract MulticallerTest is TestPlus { // Uncomment as needed to test the source files directly: // _etchMulticaller(); // _etchMulticallerWithSender(); - // _etchMulticallerWithSigner(); + _etchMulticallerWithSigner(); _deployTargets(); } @@ -460,8 +460,9 @@ contract MulticallerTest is TestPlus { keccak256( abi.encode( keccak256( - "AggregateWithSigner(address[] targets,bytes[] data,uint256[] values,uint256 nonce,uint256 nonceSalt)" + "AggregateWithSigner(address signer,address[] targets,bytes[] data,uint256[] values,uint256 nonce,uint256 nonceSalt)" ), + t.signer, keccak256(abi.encodePacked(t.targets)), keccak256(abi.encodePacked(dataHashes)), keccak256(abi.encodePacked(t.values)), @@ -925,7 +926,10 @@ contract MulticallerTest is TestPlus { _multicallerWithSignerDomainSeparator(), keccak256( abi.encode( - keccak256("InvalidateNoncesForSigner(uint256[] nonces,uint256 nonceSalt)"), + keccak256( + "InvalidateNoncesForSigner(address signer,uint256[] nonces,uint256 nonceSalt)" + ), + signer, keccak256(abi.encodePacked(nonces)), multicallerWithSigner.nonceSaltOf(signer) ) @@ -946,7 +950,8 @@ contract MulticallerTest is TestPlus { _multicallerWithSignerDomainSeparator(), keccak256( abi.encode( - keccak256("IncrementNonceSaltForSigner(uint256 nonceSalt)"), + keccak256("IncrementNonceSaltForSigner(address signer,uint256 nonceSalt)"), + signer, multicallerWithSigner.nonceSaltOf(signer) ) )