From 2b27477ec0f9e5f0e0326d302531f93ff2c65de3 Mon Sep 17 00:00:00 2001
From: Daniel Wang <99078276+dantaik@users.noreply.github.com>
Date: Mon, 8 Apr 2024 17:42:20 +0800
Subject: [PATCH] fix(protocol): fix potential 1271 signature replay if
 proposers are smart contracts (#16665)

---
 packages/protocol/contracts/L1/TaikoData.sol  |  2 +-
 .../contracts/L1/hooks/AssignmentHook.sol     | 29 ++++++++++++++-----
 packages/protocol/test/L1/TaikoL1TestBase.sol | 20 +++++++------
 3 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/packages/protocol/contracts/L1/TaikoData.sol b/packages/protocol/contracts/L1/TaikoData.sol
index 30de200d139..48df4caf873 100644
--- 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
index 644609028ec..6c66c5e3c32 100644
--- 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,26 +145,26 @@ 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,
@@ -170,6 +172,19 @@ contract AssignmentHook is EssentialContract, IHook {
                 _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
index 248278942d2..3ad05ace406 100644
--- 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);
     }