Skip to content

Commit

Permalink
fix: add staker address to DEPOSIT typehash (#424)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ChaoticWalrus authored Feb 6, 2024
1 parent 7511559 commit 6de01c6
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/contracts/core/StrategyManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/core/StrategyManagerStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/test/integration/User.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions src/test/unit/StrategyManagerUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down

0 comments on commit 6de01c6

Please sign in to comment.