fix(protocol): fix potential 1271 signature replay if proposers are s…

…mart contracts (#16665)
taikoxyz · Apr 8, 2024 · 2b27477 · 2b27477
1 parent 4909782
commit 2b27477
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 17 deletions.
diff --git a/packages/protocol/contracts/L1/TaikoData.sol b/packages/protocol/contracts/L1/TaikoData.sol
@@ -83,7 +83,7 @@ library TaikoData {
         uint16 minTier;
         bool blobUsed;
         bytes32 parentMetaHash;
-        address sender;
+        address sender; // a.k.a proposer
     }
 
     /// @dev Struct representing transition to be proven.

diff --git a/packages/protocol/contracts/L1/hooks/AssignmentHook.sol b/packages/protocol/contracts/L1/hooks/AssignmentHook.sol
@@ -96,7 +96,9 @@ contract AssignmentHook is EssentialContract, IHook {
         // the prover, therefore, we add a string as a prefix.
 
         // msg.sender is taikoL1Address
-        bytes32 hash = hashAssignment(assignment, msg.sender, _meta.blobHash);
+        bytes32 hash = hashAssignment(
+            assignment, msg.sender, _meta.sender, _blk.assignedProver, _meta.blobHash
+        );
 
         if (!_blk.assignedProver.isValidSignatureNow(hash, assignment.signature)) {
             revert HOOK_ASSIGNMENT_INVALID_SIG();
@@ -143,33 +145,46 @@ contract AssignmentHook is EssentialContract, IHook {
     /// @notice Hashes the prover assignment.
     /// @param _assignment The prover assignment.
     /// @param _taikoL1Address The address of the TaikoL1 contract.
+    /// @param _blockProposer The block proposer address.
+    /// @param _assignedProver The assigned prover address.
     /// @param _blobHash The blob hash.
     /// @return The hash of the prover assignment.
     function hashAssignment(
         ProverAssignment memory _assignment,
         address _taikoL1Address,
+        address _blockProposer,
+        address _assignedProver,
         bytes32 _blobHash
     )
         public
         view
         returns (bytes32)
     {
-        return keccak256(
+        // split up into two parts otherwise stack is too deep
+        bytes32 hash = keccak256(
             abi.encode(
-                "PROVER_ASSIGNMENT",
-                ITaikoL1(_taikoL1Address).getConfig().chainId,
-                _taikoL1Address,
-                address(this),
                 _assignment.metaHash,
                 _assignment.parentMetaHash,
-                _blobHash,
                 _assignment.feeToken,
                 _assignment.expiry,
                 _assignment.maxBlockId,
                 _assignment.maxProposedIn,
                 _assignment.tierFees
             )
         );
+
+        return keccak256(
+            abi.encodePacked(
+                "PROVER_ASSIGNMENT",
+                ITaikoL1(_taikoL1Address).getConfig().chainId,
+                _taikoL1Address,
+                _blockProposer,
+                _assignedProver,
+                _blobHash,
+                hash,
+                address(this)
+            )
+        );
     }
 
     function _getProverFee(

diff --git a/packages/protocol/test/L1/TaikoL1TestBase.sol b/packages/protocol/test/L1/TaikoL1TestBase.sol
@@ -170,8 +170,9 @@ abstract contract TaikoL1TestBase is TaikoTest {
             signature: new bytes(0)
         });
 
-        assignment.signature =
-            _signAssignment(prover, assignment, address(L1), keccak256(new bytes(txListSize)));
+        assignment.signature = _signAssignment(
+            prover, assignment, address(L1), proposer, keccak256(new bytes(txListSize))
+        );
 
         (, TaikoData.SlotB memory b) = L1.getStateVariables();
 
@@ -293,9 +294,10 @@ abstract contract TaikoL1TestBase is TaikoTest {
     }
 
     function _signAssignment(
-        address signer,
+        address prover,
         AssignmentHook.ProverAssignment memory assignment,
         address taikoAddr,
+        address blockProposer,
         bytes32 blobHash
     )
         internal
@@ -305,17 +307,17 @@ abstract contract TaikoL1TestBase is TaikoTest {
         uint256 signerPrivateKey;
 
         // In the test suite these are the 3 which acts as provers
-        if (signer == Alice) {
+        if (prover == Alice) {
             signerPrivateKey = 0x1;
-        } else if (signer == Bob) {
+        } else if (prover == Bob) {
             signerPrivateKey = 0x2;
-        } else if (signer == Carol) {
+        } else if (prover == Carol) {
             signerPrivateKey = 0x3;
         }
 
-        (uint8 v, bytes32 r, bytes32 s) = vm.sign(
-            signerPrivateKey, assignmentHook.hashAssignment(assignment, taikoAddr, blobHash)
-        );
+        bytes32 assignmentHash =
+            assignmentHook.hashAssignment(assignment, taikoAddr, blockProposer, prover, blobHash);
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPrivateKey, assignmentHash);
         signature = abi.encodePacked(r, s, v);
     }