From 20ba45eab1a4ea1116864fcc33e24f759c5d5148 Mon Sep 17 00:00:00 2001 From: Mark Barrasso <4982406+barrasso@users.noreply.github.com> Date: Wed, 27 Oct 2021 12:00:06 -0400 Subject: [PATCH 1/2] audit fixes --- contracts/BaseDebtCache.sol | 10 ++++++++-- contracts/Wrapper.sol | 40 ++++++++++++++++++++++--------------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/contracts/BaseDebtCache.sol b/contracts/BaseDebtCache.sol index 7220d53dec..fe1c806a3c 100644 --- a/contracts/BaseDebtCache.sol +++ b/contracts/BaseDebtCache.sol @@ -206,7 +206,7 @@ contract BaseDebtCache is Owned, MixinSystemSettings, IDebtCache { return _excludedIssuedDebts(currencyKeys); } - // Returns the total sUSD debt backed by non-SNX collateral, excluding debts recorded with _excludedIssuedDebt + // Returns the total sUSD debt backed by non-SNX collateral. function totalNonSnxBackedDebt() external view returns (uint excludedDebt, bool isInvalid) { bytes32[] memory currencyKeys = issuer().availableCurrencyKeys(); (uint[] memory rates, bool ratesAreInvalid) = exchangeRates().ratesAndInvalidForCurrencies(currencyKeys); @@ -214,7 +214,11 @@ contract BaseDebtCache is Owned, MixinSystemSettings, IDebtCache { return _totalNonSnxBackedDebt(currencyKeys, rates, ratesAreInvalid); } - function _totalNonSnxBackedDebt(bytes32[] memory currencyKeys, uint[] memory rates, bool ratesAreInvalid) internal view returns (uint excludedDebt, bool isInvalid) { + function _totalNonSnxBackedDebt( + bytes32[] memory currencyKeys, + uint[] memory rates, + bool ratesAreInvalid + ) internal view returns (uint excludedDebt, bool isInvalid) { // Calculate excluded debt. // 1. MultiCollateral long debt + short debt. (uint longValue, bool anyTotalLongRateIsInvalid) = collateralManager().totalLong(); @@ -226,6 +230,8 @@ contract BaseDebtCache is Owned, MixinSystemSettings, IDebtCache { // Subtract sETH and sUSD issued by EtherWrapper. excludedDebt = excludedDebt.add(etherWrapper().totalIssuedSynths()); + // 3. WrapperFactory. + // Get the debt issued by the Wrappers. for (uint i = 0; i < currencyKeys.length; i++) { excludedDebt = excludedDebt.add(_excludedIssuedDebt[currencyKeys[i]].multiplyDecimalRound(rates[i])); } diff --git a/contracts/Wrapper.sol b/contracts/Wrapper.sol index 75c5fc4656..b1a3d64cbc 100644 --- a/contracts/Wrapper.sol +++ b/contracts/Wrapper.sol @@ -55,6 +55,7 @@ contract Wrapper is Owned, Pausable, MixinResolver, MixinSystemSettings, IWrappe currencyKey = _currencyKey; synthContractName = _synthContractName; targetSynthIssued = 0; + token.approve(address(this), uint256(-1)); } /* ========== VIEWS ========== */ @@ -273,7 +274,12 @@ contract Wrapper is Owned, Pausable, MixinResolver, MixinSystemSettings, IWrappe targetSynthIssued = _targetSynthIssued; } - function _safeTransferFrom(address _tokenAddress, address _from, address _to, uint256 _value) internal returns (bool success) { + function _safeTransferFrom( + address _tokenAddress, + address _from, + address _to, + uint256 _value + ) internal returns (bool success) { // note: both of these could be replaced with manual mstore's to reduce cost if desired bytes memory msgData = abi.encodeWithSignature("transferFrom(address,address,uint256)", _from, _to, _value); uint msgSize = msgData.length; @@ -283,23 +289,25 @@ contract Wrapper is Owned, Pausable, MixinResolver, MixinSystemSettings, IWrappe mstore(0x00, 0xff) // note: this requires tangerine whistle compatible EVM - if iszero(call(gas(), _tokenAddress, 0, add(msgData, 0x20), msgSize, 0x00, 0x20)) { revert(0, 0) } - - switch mload(0x00) - case 0xff { - // token is not fully ERC20 compatible, didn't return anything, assume it was successful - success := 1 - } - case 0x01 { - success := 1 - } - case 0x00 { - success := 0 - } - default { - // unexpected value, what could this be? + if iszero(call(gas(), _tokenAddress, 0, add(msgData, 0x20), msgSize, 0x00, 0x20)) { revert(0, 0) } + + switch mload(0x00) + case 0xff { + // token is not fully ERC20 compatible, didn't return anything, assume it was successful + success := 1 + } + case 0x01 { + success := 1 + } + case 0x00 { + success := 0 + } + default { + // unexpected value, what could this be? + revert(0, 0) + } } } From a7ae6797f333fd1c32088aa55c43db82f32188b9 Mon Sep 17 00:00:00 2001 From: Mark Barrasso <4982406+barrasso@users.noreply.github.com> Date: Wed, 27 Oct 2021 18:12:44 -0400 Subject: [PATCH 2/2] add token allowance test --- test/contracts/Wrapper.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/contracts/Wrapper.js b/test/contracts/Wrapper.js index 56bf3a1b78..10ca44eb9d 100644 --- a/test/contracts/Wrapper.js +++ b/test/contracts/Wrapper.js @@ -141,6 +141,16 @@ contract('Wrapper', async accounts => { assert.equal(await instance.resolver(), addressResolver.address); }); + it('should set the wrapper token approval', async () => { + const allowance = await weth.allowance(instance.address, instance.address); + assert.bnEqual( + allowance, + web3.utils.toBN( + '115792089237316195423570985008687907853269984665640564039457584007913129639935' + ) // uint256(-1) + ); + }); + it('should access its dependencies via the address resolver', async () => { assert.equal(await addressResolver.getAddress(toBytes32('SynthsETH')), sETHSynth.address); assert.equal(await addressResolver.getAddress(toBytes32('SynthsUSD')), sUSDSynth.address);