Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SIP-182 audit fixes #1584

Merged
merged 2 commits into from
Oct 27, 2021
Merged

SIP-182 audit fixes #1584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20ba45e
audit fixes
barrasso Oct 27, 2021
a7ae679
add token allowance test
barrasso Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions contracts/BaseDebtCache.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,19 @@ 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);

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();
Expand All @@ -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]));
}
Expand Down
40 changes: 24 additions & 16 deletions contracts/Wrapper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ========== */
Expand Down Expand Up @@ -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;
Expand All @@ -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)
}
}
}

Expand Down
10 changes: 10 additions & 0 deletions test/contracts/Wrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down