From 0345b26a7b6d1a2fddb1d804539bb3fc621026c8 Mon Sep 17 00:00:00 2001 From: tamirms Date: Fri, 9 Sep 2022 08:33:38 +0200 Subject: [PATCH] explicitly check amount of tokens deposited to bridge --- solidity/contracts/Bridge.sol | 12 +++++++++--- solidity/test/erc20.js | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/solidity/contracts/Bridge.sol b/solidity/contracts/Bridge.sol index f1f25cd..f4563b8 100644 --- a/solidity/contracts/Bridge.sol +++ b/solidity/contracts/Bridge.sol @@ -3,8 +3,10 @@ pragma solidity ^0.8.0; import "./Auth.sol"; import "./StellarAsset.sol"; +import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/utils/math/SafeMath.sol"; // Every bridge transfer has a globally unique id. // @@ -80,7 +82,7 @@ bytes32 constant WITHDRAW_ETH_ID = keccak256("withdrawETH"); // WITHDRAW_ERC20_ID is used to distinguish withdrawERC20() signatures from signatures for other bridge functions. bytes32 constant WITHDRAW_ERC20_ID = keccak256("withdrawERC20"); -contract Bridge is Auth { +contract Bridge is Auth, ReentrancyGuard { // paused is a bitmask which determines whether deposits / withdrawals are enabled on the bridge uint8 public paused; // SetPaused is emitted whenever the paused state of the bridge changes @@ -134,7 +136,7 @@ contract Bridge is Auth { address token, uint256 destination, uint256 amount - ) external { + ) external nonReentrant { require((paused & PAUSE_DEPOSITS) == 0, "deposits are paused"); require(amount > 0, "deposit amount is zero"); require(token != address(0x0), "invalid token address"); @@ -145,12 +147,16 @@ contract Bridge is Auth { if (isStellarAsset[token]) { StellarAsset(token).burn(msg.sender, amount); } else { + IERC20 tokenContract = IERC20(token); + uint256 before = tokenContract.balanceOf(address(this)); SafeERC20.safeTransferFrom( - IERC20(token), + tokenContract, msg.sender, address(this), amount ); + uint256 received = SafeMath.sub(tokenContract.balanceOf(address(this)), before); + require(amount == received, "received amount not equal to expected amount"); } } diff --git a/solidity/test/erc20.js b/solidity/test/erc20.js index 83ed1f4..e537117 100644 --- a/solidity/test/erc20.js +++ b/solidity/test/erc20.js @@ -54,6 +54,20 @@ describe("Deposit & Withdraw ERC20", function() { await setPaused(bridge, signers, domainSeparator, PAUSE_NOTHING, nextPauseNonce(), validTimestamp()); }); + it("block deposits for tokens which transfer less than expected amount", async function() { + const FeeToken = await ethers.getContractFactory("FeeToken"); + const feeToken = await FeeToken.deploy("Fee Token", "FEE", 18); + await feeToken.mint(sender.address, ethers.utils.parseEther("100.0")); + await feeToken.approve(bridge.address, ethers.utils.parseEther("300.0")); + + await setDepositAllowed(bridge, signers, domainSeparator, feeToken.address, true, 1, validTimestamp()); + expect(await bridge.depositAllowed(feeToken.address)).to.be.true; + + await expect(bridge.depositERC20( + feeToken.address, 1, ethers.utils.parseEther("1.0") + )).to.be.revertedWith("received amount not equal to expected amount"); + }); + it("block deposits for a specific ERC20 token", async function() { const blockedToken = await ERC20.deploy("Blocked Test Token", "BLOCKED", 18); await blockedToken.mint(sender.address, ethers.utils.parseEther("100.0"));