From c75212bef0b3801ecda09a89d5e30e359dcf1b63 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 22 Aug 2018 17:58:47 -0700 Subject: Add nonReentrant modifiers on functions that use getCurrentContextAddress only, add lockMutex modifier on functions that make external calls --- .../2.0.0/protocol/Exchange/MixinExchangeCore.sol | 4 +++- .../2.0.0/protocol/Exchange/MixinMatchOrders.sol | 2 +- .../protocol/Exchange/MixinSignatureValidator.sol | 3 +++ .../protocol/Exchange/MixinWrapperFunctions.sol | 23 ++++++++++------------ 4 files changed, 17 insertions(+), 15 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol index d00323b15..f02514fc6 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol @@ -56,6 +56,7 @@ contract MixinExchangeCore is /// @param targetOrderEpoch Orders created with a salt less or equal to this value will be cancelled. function cancelOrdersUpTo(uint256 targetOrderEpoch) external + nonReentrant { address makerAddress = getCurrentContextAddress(); // If this function is called via `executeTransaction`, we only update the orderEpoch for the makerAddress/msg.sender combination. @@ -88,7 +89,7 @@ contract MixinExchangeCore is bytes memory signature ) public - nonReentrant + lockMutex returns (FillResults memory fillResults) { fillResults = fillOrderInternal( @@ -104,6 +105,7 @@ contract MixinExchangeCore is /// @param order Order to cancel. Order must be OrderStatus.FILLABLE. function cancelOrder(Order memory order) public + nonReentrant { // Fetch current order status OrderInfo memory orderInfo = getOrderInfo(order); diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index 25220a673..39c47e6f9 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -50,7 +50,7 @@ contract MixinMatchOrders is bytes memory rightSignature ) public - nonReentrant + lockMutex returns (LibFillResults.MatchedFillResults memory matchedFillResults) { // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData. diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol index f30adcdb8..4eb6a2fa6 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -19,6 +19,7 @@ pragma solidity 0.4.24; import "../../utils/LibBytes/LibBytes.sol"; +import "../../utils/ReentrancyGuard/ReentrancyGuard.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; import "./interfaces/IWallet.sol"; @@ -26,6 +27,7 @@ import "./interfaces/IValidator.sol"; contract MixinSignatureValidator is + ReentrancyGuard, MSignatureValidator, MTransactions { @@ -69,6 +71,7 @@ contract MixinSignatureValidator is bool approval ) external + nonReentrant { address signerAddress = getCurrentContextAddress(); allowedValidators[signerAddress][validatorAddress] = approval; diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol index ff1cb6995..b0474b110 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol @@ -46,7 +46,7 @@ contract MixinWrapperFunctions is bytes memory signature ) public - nonReentrant + lockMutex returns (FillResults memory fillResults) { fillResults = fillOrKillOrderInternal( @@ -69,6 +69,7 @@ contract MixinWrapperFunctions is bytes memory signature ) public + nonReentrant returns (FillResults memory fillResults) { // ABI encode calldata for `fillOrder` @@ -88,14 +89,7 @@ contract MixinWrapperFunctions is fillOrderCalldata, // write output over input 128 // output size is 128 bytes ) - switch success - case 0 { - mstore(fillResults, 0) - mstore(add(fillResults, 32), 0) - mstore(add(fillResults, 64), 0) - mstore(add(fillResults, 96), 0) - } - case 1 { + if success { mstore(fillResults, mload(fillOrderCalldata)) mstore(add(fillResults, 32), mload(add(fillOrderCalldata, 32))) mstore(add(fillResults, 64), mload(add(fillOrderCalldata, 64))) @@ -117,7 +111,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - nonReentrant + lockMutex returns (FillResults memory totalFillResults) { uint256 ordersLength = orders.length; @@ -144,7 +138,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - nonReentrant + lockMutex returns (FillResults memory totalFillResults) { uint256 ordersLength = orders.length; @@ -172,6 +166,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public + nonReentrant returns (FillResults memory totalFillResults) { uint256 ordersLength = orders.length; @@ -197,7 +192,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - nonReentrant + lockMutex returns (FillResults memory totalFillResults) { bytes memory takerAssetData = orders[0].takerAssetData; @@ -242,6 +237,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public + nonReentrant returns (FillResults memory totalFillResults) { bytes memory takerAssetData = orders[0].takerAssetData; @@ -285,7 +281,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public - nonReentrant + lockMutex returns (FillResults memory totalFillResults) { bytes memory makerAssetData = orders[0].makerAssetData; @@ -338,6 +334,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public + nonReentrant returns (FillResults memory totalFillResults) { bytes memory makerAssetData = orders[0].makerAssetData; -- cgit v1.2.3