Merge pull request #56 from semiotic-ai/33-m-02-signed-messages-can-b…

…e-replayed

fix: adding chain id and deadline to proof where applicaple to avoid …

src/AllocationIDTracker.sol

@@ -47,7 +47,7 @@ contract AllocationIDTracker {
      * @dev Marks an allocation ID as used.
      * @param sender The sender of the token to receiver.
      * @param allocationID The allocation ID to mark as used.
-     * @param proof ECDSA Proof signed by the receiver's allocationID consisting of packed (sender address, allocationID, escrow contract address).
+     * @param proof ECDSA Proof signed by the receiver's allocationID consisting of packed (chainID, sender address, allocationID, escrow contract address).
      * @notice REVERT with error:
      *               - AllocationIDPreviouslyClaimed: If the (sender, allocationID) pair was previously claimed
      *               - InvalidProof: If the proof is not valid
@@ -67,8 +67,8 @@ contract AllocationIDTracker {
     }
 
     /**
-     * @dev Verifies a proof.
-     * @param proof ECDSA Proof signed by the receiver's allocationID consisting of packed (sender address, allocationID, escrow contract address).
+     * @dev Verifies a proof of allocationID ownership.
+     * @param proof ECDSA Proof signed by the receiver's allocationID consisting of packed (chainID, sender address, allocationID, escrow contract address).
      * @param sender The sender of the token to receiver.
      * @param allocationID The allocation ID to verify.
      * @notice REVERT with error:
@@ -80,7 +80,7 @@ contract AllocationIDTracker {
         address allocationID
     ) private view {
         bytes32 messageHash = keccak256(
-            abi.encodePacked(sender, allocationID, msg.sender)
+            abi.encodePacked(block.chainid, sender, allocationID, msg.sender)
         );
         bytes32 digest = ECDSA.toEthSignedMessageHash(messageHash);
         if (ECDSA.recover(digest, proof) != allocationID) {

src/Escrow.sol

@@ -308,20 +308,21 @@ contract Escrow {
     /**
      * @dev Authorizes a signer to sign RAVs for the sender.
      * @param signer Address of the authorized signer.
-     * @param proof The proof provided by the signer to authorize the sender.
+     * @param proof The proof provided by the signer to authorize the sender, consisting of packed (chainID, proof deadline, sender address).
+     * @dev The proof deadline is the timestamp at which the proof expires. The proof is susceptible to replay attacks until the deadline is reached.
      * @notice REVERT with error:
      *               - SignerAlreadyAuthorized: Signer is currently authorized for a sender
      *               - InvalidSignerProof: The provided signer proof is invalid
      */
-    function authorizeSigner(address signer, bytes calldata proof) external {
+    function authorizeSigner(address signer, uint256 proofDeadline, bytes calldata proof) external {
         if (authorizedSigners[signer].sender != address(0)) {
             revert SignerAlreadyAuthorized(
                 signer,
                 authorizedSigners[signer].sender
             );
         }
 
-        verifyAuthorizedSignerProof(proof, signer);
+        verifyAuthorizedSignerProof(proof, proofDeadline, signer);
 
         authorizedSigners[signer].sender = msg.sender;
         authorizedSigners[signer].thawEndTimestamp = 0;
@@ -501,10 +502,16 @@ contract Escrow {
      */
     function verifyAuthorizedSignerProof(
         bytes calldata proof,
+        uint256 proofDeadline,
         address signer
     ) private view {
+        // Verify that the proof deadline has not passed
+        if(block.timestamp > proofDeadline) {
+            revert InvalidSignerProof();
+        }
+
         // Generate the hash of the sender's address
-        bytes32 messageHash = keccak256(abi.encodePacked(msg.sender));
+        bytes32 messageHash = keccak256(abi.encodePacked(block.chainid, proofDeadline, msg.sender));
 
         // Generate the digest to be signed by the signer
         bytes32 digest = ECDSA.toEthSignedMessageHash(messageHash);

test/Escrow.t.sol

@@ -407,19 +407,20 @@ contract EscrowContractTest is Test {
     }
 
     function authorizeSignerWithProof(address sender, uint256 signerPivateKey, address signer) private {
-        bytes memory authSignerAuthorizesSenderProof = createAuthorizedSignerProof(sender, signerPivateKey);
+        uint256 proofDeadline = block.timestamp + 86400;
+        bytes memory authSignerAuthorizesSenderProof = createAuthorizedSignerProof(proofDeadline, sender, signerPivateKey);
 
         // Authorize the signer
         vm.prank(sender);
-        escrowContract.authorizeSigner(signer, authSignerAuthorizesSenderProof);
+        escrowContract.authorizeSigner(signer, proofDeadline, authSignerAuthorizesSenderProof);
     }
 
-    function createAuthorizedSignerProof(address sender, uint256 signerPrivateKey)
+    function createAuthorizedSignerProof(uint256 proofDeadline, address sender, uint256 signerPrivateKey)
         private
-        pure
+        view
         returns (bytes memory)
     {
-        bytes32 messageHash = keccak256(abi.encodePacked(sender));
+        bytes32 messageHash = keccak256(abi.encodePacked(block.chainid, proofDeadline, sender));
         bytes32 allocationIDdigest = ECDSA.toEthSignedMessageHash(messageHash);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPrivateKey, allocationIDdigest);
         return abi.encodePacked(r, s, v);
@@ -430,8 +431,8 @@ contract EscrowContractTest is Test {
         address sender,
         address escrowContractAddress,
         uint256 allocationIDPrivateKey
-    ) private pure returns (bytes memory) {
-        bytes32 messageHash = keccak256(abi.encodePacked(sender, allocationID, escrowContractAddress));
+    ) private view returns (bytes memory) {
+        bytes32 messageHash = keccak256(abi.encodePacked(block.chainid, sender, allocationID, escrowContractAddress));
         bytes32 allocationIDdigest = ECDSA.toEthSignedMessageHash(messageHash);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(allocationIDPrivateKey, allocationIDdigest);
         return abi.encodePacked(r, s, v);