Increased signatures security by adding chainID and address

mainnet-contracts/src/GuardianModule.sol

@@ -126,7 +126,8 @@ contract GuardianModule is AccessManaged, IGuardianModule {
         external
         view
     {
-        bytes32 signedMessageHash = LibGuardianMessages._getSkipProvisioningMessage(moduleName, skippedIndex);
+        bytes32 signedMessageHash =
+            LibGuardianMessages._getSkipProvisioningMessage(address(this), moduleName, skippedIndex);
 
         // Check the signatures
         bool validSignatures =
@@ -150,6 +151,7 @@ contract GuardianModule is AccessManaged, IGuardianModule {
     ) external view {
         // Recreate the message hash
         bytes32 signedMessageHash = LibGuardianMessages._getBeaconDepositMessageToBeSigned({
+            verifyingContract: address(this),
             pufferModuleIndex: pufferModuleIndex,
             pubKey: pubKey,
             signature: signature,
@@ -175,7 +177,7 @@ contract GuardianModule is AccessManaged, IGuardianModule {
         external
         view
     {
-        bytes32 signedMessageHash = LibGuardianMessages._getHandleBatchWithdrawalMessage(validatorInfos);
+        bytes32 signedMessageHash = LibGuardianMessages._getHandleBatchWithdrawalMessage(address(this), validatorInfos);
 
         // Check the signatures
         bool validSignatures =
@@ -196,7 +198,7 @@ contract GuardianModule is AccessManaged, IGuardianModule {
     ) external view {
         // Recreate the message hash
         bytes32 signedMessageHash =
-            LibGuardianMessages._getSetNumberOfValidatorsMessage(newNumberOfValidators, epochNumber);
+            LibGuardianMessages._getSetNumberOfValidatorsMessage(address(this), newNumberOfValidators, epochNumber);
 
         // Check the signatures
         bool validSignatures =

mainnet-contracts/src/LibGuardianMessages.sol

@@ -16,74 +16,96 @@ library LibGuardianMessages {
 
     /**
      * @notice Returns the message that the guardian's enclave needs to sign
+     * @param verifyingContract is the address of the contract that will verify the signature
      * @param pufferModuleIndex is the validator index in Puffer
      * @param signature is the BLS signature of the deposit data
      * @param withdrawalCredentials are the withdrawal credentials for this validator
      * @param depositDataRoot is the hash of the deposit data
      * @return hash of the data
      */
     function _getBeaconDepositMessageToBeSigned(
+        address verifyingContract,
         uint256 pufferModuleIndex,
         bytes memory pubKey,
         bytes memory signature,
         bytes memory withdrawalCredentials,
         bytes32 depositDataRoot
-    ) internal pure returns (bytes32) {
-        return keccak256(abi.encode(pufferModuleIndex, pubKey, withdrawalCredentials, signature, depositDataRoot))
-            .toEthSignedMessageHash();
+    ) internal view returns (bytes32) {
+        return keccak256(
+            abi.encode(
+                verifyingContract,
+                block.chainid,
+                pufferModuleIndex,
+                pubKey,
+                withdrawalCredentials,
+                signature,
+                depositDataRoot
+            )
+        ).toEthSignedMessageHash();
     }
 
     /**
      * @notice Returns the message to be signed for skip provisioning
+     * @param verifyingContract is the address of the contract that will verify the signature
      * @param moduleName is the name of the module
      * @param index is the index of the skipped validator
      * @return the message to be signed
      */
-    function _getSkipProvisioningMessage(bytes32 moduleName, uint256 index) internal pure returns (bytes32) {
+    function _getSkipProvisioningMessage(address verifyingContract, bytes32 moduleName, uint256 index)
+        internal
+        view
+        returns (bytes32)
+    {
         // All guardians use the same nonce
-        return keccak256(abi.encode(moduleName, index)).toEthSignedMessageHash();
+        return keccak256(abi.encode(verifyingContract, block.chainid, moduleName, index)).toEthSignedMessageHash();
     }
 
     /**
      * @notice Returns the message to be signed for handling the batch withdrawal
+     * @param verifyingContract is the address of the contract that will verify the signature
      * @param validatorInfos is an array of validator information
      * @return the message to be signed
      */
-    function _getHandleBatchWithdrawalMessage(StoppedValidatorInfo[] memory validatorInfos)
+    function _getHandleBatchWithdrawalMessage(address verifyingContract, StoppedValidatorInfo[] memory validatorInfos)
         internal
-        pure
+        view
         returns (bytes32)
     {
-        return keccak256(abi.encode(validatorInfos)).toEthSignedMessageHash();
+        return keccak256(abi.encode(verifyingContract, block.chainid, validatorInfos)).toEthSignedMessageHash();
     }
 
     /**
      * @notice Returns the message to be signed updating the number of validators
+     * @param verifyingContract is the address of the contract that will verify the signature
      * @param numberOfValidators is the new number of validators
      * @param epochNumber is the epoch number
      * @return the message to be signed
      */
-    function _getSetNumberOfValidatorsMessage(uint256 numberOfValidators, uint256 epochNumber)
-        internal
-        pure
-        returns (bytes32)
-    {
-        return keccak256(abi.encode(numberOfValidators, epochNumber)).toEthSignedMessageHash();
+    function _getSetNumberOfValidatorsMessage(
+        address verifyingContract,
+        uint256 numberOfValidators,
+        uint256 epochNumber
+    ) internal view returns (bytes32) {
+        return keccak256(abi.encode(verifyingContract, block.chainid, numberOfValidators, epochNumber))
+            .toEthSignedMessageHash();
     }
 
     /**
      * @notice Returns the message to be signed for the no restaking module rewards root
+     * @param verifyingContract is the address of the contract that will verify the signature
      * @param moduleName is the name of the module
      * @param root is the root of the no restaking module rewards
      * @param blockNumber is the block number of the no restaking module rewards
      * @return the message to be signed
      */
-    function _getModuleRewardsRootMessage(bytes32 moduleName, bytes32 root, uint256 blockNumber)
-        internal
-        pure
-        returns (bytes32)
-    {
-        return keccak256(abi.encode(moduleName, root, blockNumber)).toEthSignedMessageHash();
+    function _getModuleRewardsRootMessage(
+        address verifyingContract,
+        bytes32 moduleName,
+        bytes32 root,
+        uint256 blockNumber
+    ) internal view returns (bytes32) {
+        return keccak256(abi.encode(verifyingContract, block.chainid, moduleName, root, blockNumber))
+            .toEthSignedMessageHash();
     }
 }
 /* solhint-disable func-named-parameters */

mainnet-contracts/test/handlers/PufferProtocolHandler.sol

@@ -606,6 +606,7 @@ contract PufferProtocolHandler is Test {
         bytes memory withdrawalCredentials = pufferProtocol.getWithdrawalCredentials(validator.module);
 
         bytes32 digest = LibGuardianMessages._getBeaconDepositMessageToBeSigned(
+            address(pufferProtocol.GUARDIAN_MODULE()),
             pendingIdx,
             pubKey,
             mockValidatorSignature,

mainnet-contracts/test/unit/PufferProtocol.t.sol

@@ -207,7 +207,11 @@ contract PufferProtocolTest is UnitTestHelper {
             5,
             99999999,
             _getGuardianEOASignatures(
-                LibGuardianMessages._getSetNumberOfValidatorsMessage({ numberOfValidators: 5, epochNumber: 99999999 })
+                LibGuardianMessages._getSetNumberOfValidatorsMessage({
+                    verifyingContract: address(guardianModule),
+                    numberOfValidators: 5,
+                    epochNumber: 99999999
+                })
             )
         );
 
@@ -1864,6 +1868,7 @@ contract PufferProtocolTest is UnitTestHelper {
         bytes memory withdrawalCredentials = pufferProtocol.getWithdrawalCredentials(validator.module);
 
         bytes32 digest = LibGuardianMessages._getBeaconDepositMessageToBeSigned(
+            address(guardianModule),
             pendingIdx,
             pubKey,
             _validatorSignature(),
@@ -1896,7 +1901,8 @@ contract PufferProtocolTest is UnitTestHelper {
     function _getGuardianSignaturesForSkipping() internal view returns (bytes[] memory) {
         (bytes32 moduleName, uint256 pendingIdx) = pufferProtocol.getNextValidatorToProvision();
 
-        bytes32 digest = LibGuardianMessages._getSkipProvisioningMessage(moduleName, pendingIdx);
+        bytes32 digest =
+            LibGuardianMessages._getSkipProvisioningMessage(address(guardianModule), moduleName, pendingIdx);
 
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(guardian1SK, digest);
         bytes memory signature1 = abi.encodePacked(r, s, v); // note the order here is different from line above.
@@ -1920,7 +1926,7 @@ contract PufferProtocolTest is UnitTestHelper {
         view
         returns (bytes[] memory)
     {
-        bytes32 digest = LibGuardianMessages._getHandleBatchWithdrawalMessage(validatorInfos);
+        bytes32 digest = LibGuardianMessages._getHandleBatchWithdrawalMessage(address(guardianModule), validatorInfos);
 
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(guardian1SK, digest);
         bytes memory signature1 = abi.encodePacked(r, s, v); // note the order here is different from line above.

mainnet-contracts/test/unit/Timelock.t.sol

@@ -18,6 +18,8 @@ contract TimelockTest is Test {
     stETHMock public stETH;
     Timelock public timelock;
 
+    address public constant BROADCASTER = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266;
+
     function setUp() public {
         PufferDeployment memory deployment = new DeployPufETH().run();
 
@@ -33,6 +35,7 @@ contract TimelockTest is Test {
         vm.assume(caller != timelock.OPERATIONS_MULTISIG());
         vm.assume(caller != address(timelock));
         vm.assume(caller != address(accessManager));
+        vm.assume(caller != BROADCASTER);
 
         // Upgrades are forbidden
         (bool canCall, uint32 delay) =
@@ -236,10 +239,11 @@ contract TimelockTest is Test {
         assertTrue(!canCall, "should not be able to call");
     }
 
-    function test_pause_depositor_slectors(address caller) public {
+    function test_pause_depositor_selectors(address caller) public {
         vm.startPrank(timelock.pauserMultisig());
         vm.assume(caller != address(timelock));
         vm.assume(caller != address(accessManager));
+        vm.assume(caller != BROADCASTER);
 
         address[] memory targets = new address[](1);
         targets[0] = address(pufferDepositor);