From 4753aa449b565462d19dfc9d3529f3bf3dfd5aee Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Tue, 6 Feb 2024 12:31:43 -0800 Subject: [PATCH] fix: add staker address to DEPOSIT typehash This provides additional signature replay protection for the `StrategyManager.depositIntoStrategyWithSignature` method Specifically, it addresses the issue outlined in https://mirror.xyz/curiousapple.eth/pFqAdW2LiJ-6S4sg_u1z08k4vK6BCJ33LcyXpnNb8yU where some ERC1271 wallets might be vulnerable to "replays" of signatures While the theoretical "damage" would be ~zero (allowing someone to deposit and credit the deposit to a user), adding this field to the typehash seems to be best practice, at least. --- src/contracts/core/StrategyManager.sol | 2 +- src/contracts/core/StrategyManagerStorage.sol | 2 +- src/test/integration/User.t.sol | 2 +- src/test/unit/StrategyManagerUnit.t.sol | 12 ++++++------ 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/contracts/core/StrategyManager.sol b/src/contracts/core/StrategyManager.sol index 5face3eb9..b0ec52d25 100644 --- a/src/contracts/core/StrategyManager.sol +++ b/src/contracts/core/StrategyManager.sol @@ -146,7 +146,7 @@ contract StrategyManager is require(expiry >= block.timestamp, "StrategyManager.depositIntoStrategyWithSignature: signature expired"); // calculate struct hash, then increment `staker`'s nonce uint256 nonce = nonces[staker]; - bytes32 structHash = keccak256(abi.encode(DEPOSIT_TYPEHASH, strategy, token, amount, nonce, expiry)); + bytes32 structHash = keccak256(abi.encode(DEPOSIT_TYPEHASH, staker, strategy, token, amount, nonce, expiry)); unchecked { nonces[staker] = nonce + 1; } diff --git a/src/contracts/core/StrategyManagerStorage.sol b/src/contracts/core/StrategyManagerStorage.sol index 8f0f79ba3..568e6770b 100644 --- a/src/contracts/core/StrategyManagerStorage.sol +++ b/src/contracts/core/StrategyManagerStorage.sol @@ -19,7 +19,7 @@ abstract contract StrategyManagerStorage is IStrategyManager { keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); /// @notice The EIP-712 typehash for the deposit struct used by the contract bytes32 public constant DEPOSIT_TYPEHASH = - keccak256("Deposit(address strategy,address token,uint256 amount,uint256 nonce,uint256 expiry)"); + keccak256("Deposit(address staker,address strategy,address token,uint256 amount,uint256 nonce,uint256 expiry)"); // maximum length of dynamic arrays in `stakerStrategyList` mapping, for sanity's sake uint8 internal constant MAX_STAKER_STRATEGY_LIST_LENGTH = 32; diff --git a/src/test/integration/User.t.sol b/src/test/integration/User.t.sol index cc3f40f00..447fcb7b2 100644 --- a/src/test/integration/User.t.sol +++ b/src/test/integration/User.t.sol @@ -432,7 +432,7 @@ contract User_AltMethods is User { // Get signature uint256 nonceBefore = strategyManager.nonces(address(this)); bytes32 structHash = keccak256( - abi.encode(strategyManager.DEPOSIT_TYPEHASH(), strat, underlyingToken, tokenBalance, nonceBefore, expiry) + abi.encode(strategyManager.DEPOSIT_TYPEHASH(), address(this), strat, underlyingToken, tokenBalance, nonceBefore, expiry) ); bytes32 digestHash = keccak256(abi.encodePacked("\x19\x01", strategyManager.domainSeparator(), structHash)); bytes memory signature = bytes(abi.encodePacked(digestHash)); // dummy sig data diff --git a/src/test/unit/StrategyManagerUnit.t.sol b/src/test/unit/StrategyManagerUnit.t.sol index df1119f3e..5b7cb2648 100644 --- a/src/test/unit/StrategyManagerUnit.t.sol +++ b/src/test/unit/StrategyManagerUnit.t.sol @@ -153,7 +153,7 @@ contract StrategyManagerUnitTests is EigenLayerUnitTestSetup, IStrategyManagerEv { bytes32 structHash = keccak256( - abi.encode(strategyManager.DEPOSIT_TYPEHASH(), dummyStrat, dummyToken, amount, nonceBefore, expiry) + abi.encode(strategyManager.DEPOSIT_TYPEHASH(), staker, dummyStrat, dummyToken, amount, nonceBefore, expiry) ); bytes32 digestHash = keccak256(abi.encodePacked("\x19\x01", strategyManager.domainSeparator(), structHash)); @@ -511,7 +511,7 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa { bytes32 structHash = keccak256( - abi.encode(strategyManager.DEPOSIT_TYPEHASH(), strategy, token, amount, nonceBefore, expiry) + abi.encode(strategyManager.DEPOSIT_TYPEHASH(), staker, strategy, token, amount, nonceBefore, expiry) ); bytes32 digestHash = keccak256(abi.encodePacked("\x19\x01", strategyManager.domainSeparator(), structHash)); @@ -582,7 +582,7 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa { bytes32 structHash = keccak256( - abi.encode(strategyManager.DEPOSIT_TYPEHASH(), strategy, token, amount, nonceBefore, expiry) + abi.encode(strategyManager.DEPOSIT_TYPEHASH(), staker, strategy, token, amount, nonceBefore, expiry) ); bytes32 digestHash = keccak256(abi.encodePacked("\x19\x01", strategyManager.domainSeparator(), structHash)); @@ -638,7 +638,7 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa { bytes32 structHash = keccak256( - abi.encode(strategyManager.DEPOSIT_TYPEHASH(), dummyStrat, revertToken, amount, nonceBefore, expiry) + abi.encode(strategyManager.DEPOSIT_TYPEHASH(), staker, dummyStrat, revertToken, amount, nonceBefore, expiry) ); bytes32 digestHash = keccak256(abi.encodePacked("\x19\x01", strategyManager.domainSeparator(), structHash)); @@ -713,7 +713,7 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa { bytes32 structHash = keccak256( - abi.encode(strategyManager.DEPOSIT_TYPEHASH(), strategy, token, amount, nonceBefore, expiry) + abi.encode(strategyManager.DEPOSIT_TYPEHASH(), staker, strategy, token, amount, nonceBefore, expiry) ); bytes32 digestHash = keccak256(abi.encodePacked("\x19\x01", strategyManager.domainSeparator(), structHash)); @@ -753,7 +753,7 @@ contract StrategyManagerUnitTests_depositIntoStrategyWithSignature is StrategyMa { bytes32 structHash = keccak256( - abi.encode(strategyManager.DEPOSIT_TYPEHASH(), strategy, token, amount, nonceBefore, expiry) + abi.encode(strategyManager.DEPOSIT_TYPEHASH(), staker, strategy, token, amount, nonceBefore, expiry) ); bytes32 digestHash = keccak256(abi.encodePacked("\x19\x01", strategyManager.domainSeparator(), structHash));