From df780b55a0041de2965d39f8a862b0b6469eb50a Mon Sep 17 00:00:00 2001 From: Thrilok Kumar Date: Wed, 7 Oct 2020 20:44:23 +0530 Subject: [PATCH 1/4] Added nonReentrant guard to deposit() in token pool --- contracts/pools/erc20.sol | 2 +- contracts/pools/eth.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/pools/erc20.sol b/contracts/pools/erc20.sol index 49900d1..e813ed7 100644 --- a/contracts/pools/erc20.sol +++ b/contracts/pools/erc20.sol @@ -118,7 +118,7 @@ contract PoolToken is ReentrancyGuard, ERC20Pausable, DSMath { * @param tknAmt token amount * @return mintAmt amount of wrap token minted */ - function deposit(uint tknAmt) external payable whenNotPaused returns (uint mintAmt) { + function deposit(uint tknAmt) external payable nonReentrant whenNotPaused returns (uint mintAmt) { require(msg.value == 0, "non-eth-pool"); uint _tokenBal = wdiv(totalSupply(), exchangeRate); uint _newTknBal = add(_tokenBal, tknAmt); diff --git a/contracts/pools/eth.sol b/contracts/pools/eth.sol index 71a3fdb..143a69a 100644 --- a/contracts/pools/eth.sol +++ b/contracts/pools/eth.sol @@ -115,7 +115,7 @@ contract PoolETH is ReentrancyGuard, ERC20Pausable, DSMath { * @param tknAmt token amount * @return mintAmt amount of wrap token minted */ - function deposit(uint tknAmt) external whenNotPaused payable returns (uint mintAmt) { + function deposit(uint tknAmt) external nonReentrant whenNotPaused payable returns (uint mintAmt) { require(tknAmt == msg.value, "unmatched-amount"); uint _tokenBal = wdiv(totalSupply(), exchangeRate); uint _newTknBal = add(_tokenBal, tknAmt); From d3b423ba62438ab0e57f97f2bdf01e1158331dbe Mon Sep 17 00:00:00 2001 From: Thrilok Kumar Date: Wed, 7 Oct 2020 20:46:17 +0530 Subject: [PATCH 2/4] Added sanity checks in settle() token pools --- contracts/pools/erc20.sol | 1 + contracts/pools/eth.sol | 1 + 2 files changed, 2 insertions(+) diff --git a/contracts/pools/erc20.sol b/contracts/pools/erc20.sol index e813ed7..93b88c2 100644 --- a/contracts/pools/erc20.sol +++ b/contracts/pools/erc20.sol @@ -105,6 +105,7 @@ contract PoolToken is ReentrancyGuard, ERC20Pausable, DSMath { * @param _data array of connector's function calldata */ function settle(address[] calldata _targets, bytes[] calldata _data) external isChief { + require(_targets.length != 0, "targets-length-zero"); require(_targets.length == _data.length , "array-length-invalid"); require(registry.checkSettleLogics(address(this), _targets), "not-logic"); for (uint i = 0; i < _targets.length; i++) { diff --git a/contracts/pools/eth.sol b/contracts/pools/eth.sol index 143a69a..c52367f 100644 --- a/contracts/pools/eth.sol +++ b/contracts/pools/eth.sol @@ -102,6 +102,7 @@ contract PoolETH is ReentrancyGuard, ERC20Pausable, DSMath { * @param _data array of connector's function calldata */ function settle(address[] calldata _targets, bytes[] calldata _data) external isChief { + require(_targets.length != 0, "targets-length-zero"); require(_targets.length == _data.length , "array-length-invalid"); require(registry.checkSettleLogics(address(this), _targets), "not-logic"); for (uint i = 0; i < _targets.length; i++) { From 4bff80010d258446f24153cb3ce026956cef06e7 Mon Sep 17 00:00:00 2001 From: Thrilok Kumar Date: Wed, 7 Oct 2020 20:49:11 +0530 Subject: [PATCH 3/4] Replaced `transfer()` with `sendValue()` --- contracts/pools/eth.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/pools/eth.sol b/contracts/pools/eth.sol index c52367f..fc1d57a 100644 --- a/contracts/pools/eth.sol +++ b/contracts/pools/eth.sol @@ -4,6 +4,7 @@ pragma experimental ABIEncoderV2; import "@openzeppelin/contracts/token/ERC20/ERC20Pausable.sol"; import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; +import "@openzeppelin/contracts/utils/Address.sol"; import { DSMath } from "../libs/safeMath.sol"; @@ -146,7 +147,7 @@ contract PoolETH is ReentrancyGuard, ERC20Pausable, DSMath { require(wdAmt <= address(this).balance, "not-enough-liquidity-available"); _burn(msg.sender, _burnAmt); - payable(target).transfer(wdAmt); + Address.sendValue(payable(target), wdAmt); emit LogWithdraw(msg.sender, wdAmt, _burnAmt); } @@ -159,7 +160,7 @@ contract PoolETH is ReentrancyGuard, ERC20Pausable, DSMath { function withdrawFee(uint wdAmt) external { require(msg.sender == instaIndex.master(), "not-master"); if (wdAmt > feeAmt) wdAmt = feeAmt; - msg.sender.transfer(wdAmt); + Address.sendValue(payable(msg.sender), wdAmt); feeAmt = sub(feeAmt, wdAmt); emit LogWithdrawFee(wdAmt); } From 6bbe1f80c88c08248c2f61cdb625bbc8c4b79833 Mon Sep 17 00:00:00 2001 From: Thrilok Kumar Date: Wed, 7 Oct 2020 20:52:22 +0530 Subject: [PATCH 4/4] Fixed Deflationary/Rebasing Token bug in token pool --- contracts/pools/erc20.sol | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/contracts/pools/erc20.sol b/contracts/pools/erc20.sol index 93b88c2..11d587b 100644 --- a/contracts/pools/erc20.sol +++ b/contracts/pools/erc20.sol @@ -121,13 +121,17 @@ contract PoolToken is ReentrancyGuard, ERC20Pausable, DSMath { */ function deposit(uint tknAmt) external payable nonReentrant whenNotPaused returns (uint mintAmt) { require(msg.value == 0, "non-eth-pool"); + require(tknAmt != 0, "tknAmt-is-zero"); uint _tokenBal = wdiv(totalSupply(), exchangeRate); uint _newTknBal = add(_tokenBal, tknAmt); require(_newTknBal < registry.poolCap(address(this)), "pool-cap-reached"); + uint initalBal = baseToken.balanceOf(address(this)); baseToken.safeTransferFrom(msg.sender, address(this), tknAmt); - mintAmt = wmul(tknAmt, exchangeRate); + uint finalBal = baseToken.balanceOf(address(this)); + uint _tknAmt = sub(finalBal, initalBal); + mintAmt = wmul(_tknAmt, exchangeRate); _mint(msg.sender, mintAmt); - emit LogDeposit(msg.sender, tknAmt, mintAmt); + emit LogDeposit(msg.sender, _tknAmt, mintAmt); } /**