From 56c3c29febe6264eee7f1c3f1ed8f1dc7c4685c6 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 22 Aug 2018 18:00:01 -0700
Subject: Update ReentrantERC20Token with new functions and check that revert
is occuring for correct reason
---
.../ReentrantERC20Token/ReentrantERC20Token.sol | 110 ++++++++++++++-------
1 file changed, 75 insertions(+), 35 deletions(-)
diff --git a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
index f5c98177f..8d575d09a 100644
--- a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
+++ b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
@@ -19,6 +19,7 @@
pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
+import "../../utils/LibBytes/LibBytes.sol";
import "../../tokens/ERC20Token/ERC20Token.sol";
import "../../protocol/Exchange/interfaces/IExchange.sol";
import "../../protocol/Exchange/libs/LibOrder.sol";
@@ -27,9 +28,17 @@ import "../../protocol/Exchange/libs/LibOrder.sol";
contract ReentrantERC20Token is
ERC20Token
{
+
+ using LibBytes for bytes;
+
// solhint-disable-next-line var-name-mixedcase
IExchange internal EXCHANGE;
+ bytes internal constant REENTRANCY_ILLEGAL_REVERT_REASON = abi.encodeWithSelector(
+ bytes4(keccak256("Error(string)")),
+ "REENTRANCY_ILLEGAL"
+ );
+
// All of these functions are potentially vulnerable to reentrancy
enum ExchangeFunction {
FILL_ORDER,
@@ -42,7 +51,10 @@ contract ReentrantERC20Token is
MARKET_BUY_ORDERS_NO_THROW,
MARKET_SELL_ORDERS,
MARKET_SELL_ORDERS_NO_THROW,
- MATCH_ORDERS
+ MATCH_ORDERS,
+ CANCEL_ORDER,
+ CANCEL_ORDERS_UP_TO,
+ SET_SIGNATURE_VALIDATOR_APPROVAL
}
uint8 internal currentFunctionId = 0;
@@ -75,99 +87,127 @@ contract ReentrantERC20Token is
{
// This order would normally be invalid, but it will be used strictly for testing reentrnacy.
// Any reentrancy checks will happen before any other checks that invalidate the order.
- LibOrder.Order memory order = LibOrder.Order({
- makerAddress: address(0),
- takerAddress: address(0),
- feeRecipientAddress: address(0),
- senderAddress: address(0),
- makerAssetAmount: 0,
- takerAssetAmount: 0,
- makerFee: 0,
- takerFee: 0,
- expirationTimeSeconds: 0,
- salt: 0,
- makerAssetData: "",
- takerAssetData: ""
- });
+ LibOrder.Order memory order;
// Initialize remaining null parameters
- bytes memory signature = "";
- LibOrder.Order[] memory orders = new LibOrder.Order[](1);
- orders[0] = order;
- uint256[] memory takerAssetFillAmounts = new uint256[](1);
- takerAssetFillAmounts[0] = 0;
- bytes[] memory signatures = new bytes[](1);
- signatures[0] = signature;
-
- // Attempt to reenter Exchange by calling function with currentFunctionId
+ bytes memory signature;
+ LibOrder.Order[] memory orders;
+ uint256[] memory takerAssetFillAmounts;
+ bytes[] memory signatures;
+ bytes memory calldata;
+
+ // Create calldata for function that corresponds to currentFunctionId
if (currentFunctionId == uint8(ExchangeFunction.FILL_ORDER)) {
- EXCHANGE.fillOrder(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.fillOrder.selector,
order,
0,
signature
);
} else if (currentFunctionId == uint8(ExchangeFunction.FILL_OR_KILL_ORDER)) {
- EXCHANGE.fillOrKillOrder(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.fillOrKillOrder.selector,
order,
0,
signature
);
} else if (currentFunctionId == uint8(ExchangeFunction.FILL_ORDER_NO_THROW)) {
- EXCHANGE.fillOrderNoThrow(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.fillOrderNoThrow.selector,
order,
0,
signature
);
} else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_ORDERS)) {
- EXCHANGE.batchFillOrders(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.batchFillOrders.selector,
orders,
takerAssetFillAmounts,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_OR_KILL_ORDERS)) {
- EXCHANGE.batchFillOrKillOrders(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.batchFillOrKillOrders.selector,
orders,
takerAssetFillAmounts,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_ORDERS_NO_THROW)) {
- EXCHANGE.batchFillOrdersNoThrow(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.batchFillOrdersNoThrow.selector,
orders,
takerAssetFillAmounts,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.MARKET_BUY_ORDERS)) {
- EXCHANGE.marketBuyOrders(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.marketBuyOrders.selector,
orders,
0,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.MARKET_BUY_ORDERS_NO_THROW)) {
- EXCHANGE.marketBuyOrdersNoThrow(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.marketBuyOrdersNoThrow.selector,
orders,
0,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.MARKET_SELL_ORDERS)) {
- EXCHANGE.marketSellOrders(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.marketSellOrders.selector,
orders,
0,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.MARKET_SELL_ORDERS_NO_THROW)) {
- EXCHANGE.marketSellOrdersNoThrow(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.marketSellOrdersNoThrow.selector,
orders,
0,
signatures
);
} else if (currentFunctionId == uint8(ExchangeFunction.MATCH_ORDERS)) {
- EXCHANGE.matchOrders(
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.matchOrders.selector,
order,
order,
signature,
signature
);
+ } else if (currentFunctionId == uint8(ExchangeFunction.CANCEL_ORDER)) {
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.cancelOrder.selector,
+ order
+ );
+ } else if (currentFunctionId == uint8(ExchangeFunction.CANCEL_ORDERS_UP_TO)) {
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.cancelOrdersUpTo.selector,
+ 0
+ );
+ } else if (currentFunctionId == uint8(ExchangeFunction.SET_SIGNATURE_VALIDATOR_APPROVAL)) {
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.setSignatureValidatorApproval.selector,
+ address(0),
+ false
+ );
}
+
+ // Call Exchange function, swallow error
+ address(EXCHANGE).call(calldata);
+
+ // Revert reason is 100 bytes
+ bytes memory returnData = new bytes(100);
+
+ // Copy return data
+ assembly {
+ returndatacopy(add(returnData, 32), 0, 100)
+ }
+
+ // Revert if function reverted with REENTRANCY_ILLEGAL error
+ require(!REENTRANCY_ILLEGAL_REVERT_REASON.equals(returnData));
+
+ // Transfer will return true if function failed for any other reason
return true;
}
}
\ No newline at end of file
--
cgit v1.2.3