Fix issue: Potential Signature Replay Attack (Add expiry time)

bosagora · May 8, 2024 · af6ade6 · af6ade6
1 parent e43a3df
commit af6ade6
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 9 deletions.
diff --git a/packages/contracts/contracts/BIP20/BIP20DelegatedTransfer.sol b/packages/contracts/contracts/BIP20/BIP20DelegatedTransfer.sol
@@ -26,10 +26,12 @@ contract BIP20DelegatedTransfer is BIP20, IBIP20DelegatedTransfer {
         address from,
         address to,
         uint256 amount,
+        uint256 expiry,
         bytes calldata signature
     ) external override returns (bool) {
-        bytes32 dataHash = keccak256(abi.encode(block.chainid, address(this), from, to, amount, nonce[from]));
+        bytes32 dataHash = keccak256(abi.encode(block.chainid, address(this), from, to, amount, nonce[from], expiry));
         require(ECDSA.recover(ECDSA.toEthSignedMessageHash(dataHash), signature) == from, "Invalid signature");
+        require(expiry > block.timestamp, "Expired signature");
 
         super._transfer(from, to, amount);
         nonce[from]++;

diff --git a/packages/contracts/contracts/BIP20/IBIP20DelegatedTransfer.sol b/packages/contracts/contracts/BIP20/IBIP20DelegatedTransfer.sol
@@ -10,6 +10,7 @@ interface IBIP20DelegatedTransfer is IBIP20 {
         address from,
         address to,
         uint256 amount,
+        uint256 expiry,
         bytes calldata signature
     ) external returns (bool);
 }
diff --git a/packages/contracts/src/utils/ContractUtils.ts b/packages/contracts/src/utils/ContractUtils.ts
@@ -96,11 +96,12 @@ export class ContractUtils {
         from: string,
         to: string,
         amount: BigNumberish,
-        nonce: BigNumberish
+        nonce: BigNumberish,
+        expiry: number
     ): Uint8Array {
         const encodedResult = defaultAbiCoder.encode(
-            ["uint256", "address", "address", "address", "uint256", "uint256"],
-            [chainId, tokenAddress, from, to, amount, nonce]
+            ["uint256", "address", "address", "address", "uint256", "uint256", "uint256"],
+            [chainId, tokenAddress, from, to, amount, nonce, expiry]
         );
         return arrayify(keccak256(encodedResult));
     }

diff --git a/packages/contracts/test/DelegatedTransfer.test.ts b/packages/contracts/test/DelegatedTransfer.test.ts
@@ -158,33 +158,56 @@ describe("Test for LYT token", () => {
     it("Transfer from account4 to account5 - Invalid Signature", async () => {
         const amount = BOACoin.make(500).value;
         const nonce = await token.nonceOf(account4.address);
+        const expiry = ContractUtils.getTimeStamp() + 12 * 5;
         const message = ContractUtils.getTransferMessage(
             ethers.provider.network.chainId,
             token.address,
             account4.address,
             account5.address,
             amount,
-            nonce
+            nonce,
+            expiry
         );
         const signature = ContractUtils.signMessage(account3, message);
-        await expect(token.delegatedTransfer(account4.address, account5.address, amount, signature)).to.be.revertedWith(
-            "Invalid signature"
+        await expect(
+            token.delegatedTransfer(account4.address, account5.address, amount, expiry, signature)
+        ).to.be.revertedWith("Invalid signature");
+    });
+
+    it("Transfer from account4 to account5 - Expired signature", async () => {
+        const amount = BOACoin.make(500).value;
+        const nonce = await token.nonceOf(account4.address);
+        const expiry = ContractUtils.getTimeStamp() - 12;
+        const message = ContractUtils.getTransferMessage(
+            ethers.provider.network.chainId,
+            token.address,
+            account4.address,
+            account5.address,
+            amount,
+            nonce,
+            expiry
         );
+        const signature = ContractUtils.signMessage(account4, message);
+        await expect(
+            token.delegatedTransfer(account4.address, account5.address, amount, expiry, signature)
+        ).to.be.revertedWith("Expired signature");
     });
 
     it("Transfer from account4 to account5", async () => {
         const amount = BOACoin.make(500).value;
         const nonce = await token.nonceOf(account4.address);
+        const expiry = ContractUtils.getTimeStamp() + 12 * 5;
         const message = ContractUtils.getTransferMessage(
             ethers.provider.network.chainId,
             token.address,
             account4.address,
             account5.address,
             amount,
-            nonce
+            nonce,
+            expiry
         );
         const signature = ContractUtils.signMessage(account4, message);
-        await token.delegatedTransfer(account4.address, account5.address, amount, signature);
+        await token.delegatedTransfer(account4.address, account5.address, amount, expiry, signature);
 
         assert.deepStrictEqual(await token.balanceOf(account5.address), amount);
     });