Add test for malicious erc20

juliamist · Dec 23, 2021 · 3fd78fc · 3fd78fc
1 parent 380ecc3
commit 3fd78fc
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 2 deletions.
diff --git a/contracts/contracts/dev-contracts/DummyERC20BytesTransferReturnValue.sol b/contracts/contracts/dev-contracts/DummyERC20BytesTransferReturnValue.sol
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: UNLICENSED
+
+pragma solidity ^0.7.0;
+
+contract DummyERC20BytesTransferReturnValue {
+    bytes returnValue;
+
+    constructor(bytes memory _returnValue) {
+        returnValue = _returnValue;
+    }
+
+    function transfer(address _recipient, uint256 _amount) external view returns (bytes memory) {
+        // Hack to prevent Solidity warnings
+        _recipient;
+        _amount;
+
+        return returnValue;
+    }
+}
diff --git a/contracts/contracts/dev-contracts/DummyERC20NoTransferReturnValue.sol b/contracts/contracts/dev-contracts/DummyERC20NoTransferReturnValue.sol
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: UNLICENSED
+
+pragma solidity ^0.7.0;
+
+contract DummyERC20NoTransferReturnValue {
+    function transfer(address recipient, uint256 amount) external {}
+}
diff --git a/contracts/test/unit_tests/zksync_test.ts b/contracts/test/unit_tests/zksync_test.ts
@@ -11,6 +11,11 @@ const { getCallRevertReason, IERC20_INTERFACE, DEFAULT_REVERT_REASON } = require
 import * as zksync from 'zksync';
 import {
     ZkSync,
+    TestnetERC20Token,
+    DummyERC20NoTransferReturnValue,
+    DummyERC20NoTransferReturnValueFactory,
+    DummyERC20BytesTransferReturnValue,
+    DummyERC20BytesTransferReturnValueFactory,
     ZkSyncProcessOpUnitTest,
     ZkSyncProcessOpUnitTestFactory,
     ZKSyncSignatureUnitTest,
@@ -281,16 +286,39 @@ describe('zkSync withdraw unit tests', function () {
     this.timeout(50000);
 
     let zksyncContract: ZkSync;
-    let tokenContract;
+    let tokenContract: TestnetERC20Token;
     let incorrectTokenContract;
-    let ethProxy;
+    let ethProxy: ETHProxy;
+    let EOA_Address: string;
+    let tokenNoTransferReturnValue: DummyERC20NoTransferReturnValue;
+    let tokenBytesTransferReturnValue: DummyERC20BytesTransferReturnValue;
     before(async () => {
         [wallet] = await hardhat.ethers.getSigners();
+        EOA_Address = wallet.address;
+
         const contracts = readProductionContracts();
         contracts.zkSync = readContractCode('dev-contracts/ZkSyncWithdrawalUnitTest');
         const deployer = new Deployer({ deployWallet: wallet, contracts });
         await deployer.deployAll({ gasLimit: 6500000 });
 
+        const tokenNoTransferReturnValueFactory = await hardhat.ethers.getContractFactory(
+            'DummyERC20NoTransferReturnValue'
+        );
+        const tokenNoTransferReturnValueContract = await tokenNoTransferReturnValueFactory.deploy();
+        tokenNoTransferReturnValue = DummyERC20NoTransferReturnValueFactory.connect(
+            tokenNoTransferReturnValueContract.address,
+            tokenNoTransferReturnValueContract.signer
+        );
+
+        const tokenBytesTransferReturnValueFactory = await hardhat.ethers.getContractFactory(
+            'DummyERC20BytesTransferReturnValue'
+        );
+        const tokenBytesTransferReturnValueContract = await tokenBytesTransferReturnValueFactory.deploy('0xDEADBEEF');
+        tokenBytesTransferReturnValue = DummyERC20BytesTransferReturnValueFactory.connect(
+            tokenBytesTransferReturnValueContract.address,
+            tokenBytesTransferReturnValueContract.signer
+        );
+
         zksyncContract = ZkSyncWithdrawalUnitTestFactory.connect(deployer.addresses.ZkSync, wallet);
 
         const tokenContractFactory = await hardhat.ethers.getContractFactory('TestnetERC20Token');
@@ -299,6 +327,9 @@ describe('zkSync withdraw unit tests', function () {
 
         const govContract = deployer.governanceContract(wallet);
         await govContract.addToken(tokenContract.address);
+        await govContract.addToken(EOA_Address);
+        await govContract.addToken(tokenNoTransferReturnValue.address);
+        await govContract.addToken(tokenBytesTransferReturnValue.address);
 
         ethProxy = new ETHProxy(wallet.provider, {
             mainContract: zksyncContract.address,
@@ -419,6 +450,30 @@ describe('zkSync withdraw unit tests', function () {
         );
         expect(revertReason, 'wrong revert reason').eq('1i');
     });
+
+    it('Withdraw token with empty return value', async () => {
+        const balanceBefore = await zksyncContract.getPendingBalance(EOA_Address, tokenNoTransferReturnValue.address);
+        await zksyncContract.withdrawOrStoreExternal(3, EOA_Address, 1);
+        const balanceAfter = await zksyncContract.getPendingBalance(EOA_Address, tokenNoTransferReturnValue.address);
+        expect(balanceAfter.eq(balanceBefore));
+    });
+
+    it('Should save pending balance for token without bytecode', async () => {
+        const balanceBefore = await zksyncContract.getPendingBalance(EOA_Address, EOA_Address);
+        await zksyncContract.withdrawOrStoreExternal(2, EOA_Address, 1);
+        const balanceAfter = await zksyncContract.getPendingBalance(EOA_Address, EOA_Address);
+        expect(balanceAfter.eq(balanceBefore.add(1)));
+    });
+
+    it('Should save pending balance for token with incorrect return value', async () => {
+        const balanceBefore = await zksyncContract.getPendingBalance(
+            EOA_Address,
+            tokenBytesTransferReturnValue.address
+        );
+        await zksyncContract.withdrawOrStoreExternal(4, EOA_Address, 1);
+        const balanceAfter = await zksyncContract.getPendingBalance(EOA_Address, tokenBytesTransferReturnValue.address);
+        expect(balanceAfter.eq(balanceBefore.add(1)));
+    });
 });
 
 describe('zkSync auth pubkey onchain unit tests', function () {