Fix Exchange reentrancy tests

This commit is contained in:
Amir Bandeali
2019-08-25 18:29:04 -07:00
parent 793e338dd3
commit 1400ceb4e8
2 changed files with 35 additions and 17 deletions

View File

@@ -45,7 +45,7 @@ contract IsolatedExchange is
Exchange(1337) Exchange(1337)
{} {}
/// @dev Overriden to only log arguments and revert on certain assetDatas. /// @dev Overridden to only log arguments and revert on certain assetDatas.
function _dispatchTransferFrom( function _dispatchTransferFrom(
bytes32 orderHash, bytes32 orderHash,
bytes memory assetData, bytes memory assetData,
@@ -69,7 +69,7 @@ contract IsolatedExchange is
} }
} }
/// @dev Overriden to simplify signature validation. /// @dev Overridden to simplify signature validation.
/// Unfortunately, this is `view`, so it can't log arguments. /// Unfortunately, this is `view`, so it can't log arguments.
function _isValidOrderWithHashSignature( function _isValidOrderWithHashSignature(
LibOrder.Order memory order, LibOrder.Order memory order,

View File

@@ -46,10 +46,10 @@ contract ReentrancyTester is
/// @dev Calls a public function to check if it is reentrant. /// @dev Calls a public function to check if it is reentrant.
function isReentrant(bytes calldata fnCallData) function isReentrant(bytes calldata fnCallData)
external external
nonReentrant
returns (bool isReentrant) returns (bool isReentrant)
{ {
bytes memory callData = abi.encodeWithSelector(this.testReentrantFunction.selector, fnCallData); (bool didSucceed, bytes memory resultData) = address(this).delegatecall(fnCallData);
(bool didSucceed, bytes memory resultData) = address(this).delegatecall(callData);
if (didSucceed) { if (didSucceed) {
isReentrant = true; isReentrant = true;
} else { } else {
@@ -61,18 +61,36 @@ contract ReentrancyTester is
} }
} }
/// @dev Calls a public function to check if it is reentrant. /// @dev Overridden to revert on unsuccessful fillOrder call.
/// Because this function uses the `nonReentrant` modifier, if function fillOrderNoThrow(
/// the function being called is also guarded by the `nonReentrant` modifier, LibOrder.Order memory order,
/// it will revert when it returns. uint256 takerAssetFillAmount,
function testReentrantFunction(bytes calldata fnCallData) bytes memory signature
external )
nonReentrant public
returns (LibFillResults.FillResults memory fillResults)
{ {
address(this).delegatecall(fnCallData); // ABI encode calldata for `fillOrder`
bytes memory fillOrderCalldata = abi.encodeWithSelector(
IExchangeCore(address(0)).fillOrder.selector,
order,
takerAssetFillAmount,
signature
);
(bool didSucceed, bytes memory returnData) = address(this).delegatecall(fillOrderCalldata);
if (didSucceed) {
assert(returnData.length == 128);
fillResults = abi.decode(returnData, (LibFillResults.FillResults));
return fillResults;
}
// Revert and rethrow error if unsuccessful
assembly {
revert(add(returnData, 32), mload(returnData))
}
} }
/// @dev Overriden to always succeed. /// @dev Overridden to always succeed.
function _fillOrder( function _fillOrder(
LibOrder.Order memory order, LibOrder.Order memory order,
uint256 takerAssetFillAmount, uint256 takerAssetFillAmount,
@@ -87,7 +105,7 @@ contract ReentrancyTester is
fillResults.takerFeePaid = order.takerFee; fillResults.takerFeePaid = order.takerFee;
} }
/// @dev Overriden to always succeed. /// @dev Overridden to always succeed.
function _fillOrKillOrder( function _fillOrKillOrder(
LibOrder.Order memory order, LibOrder.Order memory order,
uint256 takerAssetFillAmount, uint256 takerAssetFillAmount,
@@ -114,7 +132,7 @@ contract ReentrancyTester is
return resultData; return resultData;
} }
/// @dev Overriden to always succeed. /// @dev Overridden to always succeed.
function _batchMatchOrders( function _batchMatchOrders(
LibOrder.Order[] memory leftOrders, LibOrder.Order[] memory leftOrders,
LibOrder.Order[] memory rightOrders, LibOrder.Order[] memory rightOrders,
@@ -144,7 +162,7 @@ contract ReentrancyTester is
} }
} }
/// @dev Overriden to always succeed. /// @dev Overridden to always succeed.
function _matchOrders( function _matchOrders(
LibOrder.Order memory leftOrder, LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder, LibOrder.Order memory rightOrder,
@@ -169,7 +187,7 @@ contract ReentrancyTester is
}); });
} }
/// @dev Overriden to do nothing. /// @dev Overridden to do nothing.
function _cancelOrder(LibOrder.Order memory order) function _cancelOrder(LibOrder.Order memory order)
internal internal
{} {}