EDT-2912 Prevent direct calls to the delegate contracts, meaning 1155…

… and XXXX funds cannot be sent to that address only to/through the proxy receiver.
pelith · Apr 8, 2019 · 0d95695 · 0d95695
1 parent 45685c9
commit 0d95695
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 9 deletions.
diff --git a/contracts/ProxyReceiver/Delegates/ERC1155ReceiverDelegate.sol b/contracts/ProxyReceiver/Delegates/ERC1155ReceiverDelegate.sol
@@ -15,12 +15,16 @@ contract ERC1155ReceiverDelegate is ProxyReceiverStorage_001_ERC1155MockReceiver
     bytes4 constant public ERC1155_BATCH_RECEIVED = 0xbc197c81;
     bytes4 constant public NOT_ERC1155_RECEIVED = 0xa23a6e60; // Some random value
 
-    function setShouldReject(bool _value) public {
+    function setShouldReject(bool _value) external {
+        require(address(this) == proxy, "Direct call: setShouldReject");
+
         shouldReject = _value;
     }
 
     function onERC1155Received(address _operator, address _from, uint256 _id, uint256 _value, bytes calldata _data) external returns(bytes4) {
-        (_operator); (_from); (_id); (_value); (_data);
+        (_operator); (_from); (_id); (_value); (_data);  // solidity, be quiet please
+
+        require(address(this) == proxy, "Direct call: onERC1155Received");
 
         if (shouldReject == true) {
             return NOT_ERC1155_RECEIVED;
@@ -30,7 +34,9 @@ contract ERC1155ReceiverDelegate is ProxyReceiverStorage_001_ERC1155MockReceiver
     }
 
     function onERC1155BatchReceived(address _operator, address _from, uint256[] calldata _ids, uint256[] calldata _values, bytes calldata _data) external returns(bytes4) {
-        (_operator); (_from); (_ids); (_values); (_data);
+        (_operator); (_from); (_ids); (_values); (_data); // solidity, be quiet please
+
+        require(address(this) == proxy, "Direct call: onERC1155BatchReceived");
 
         if (shouldReject == true) {
             return NOT_ERC1155_RECEIVED;

diff --git a/contracts/ProxyReceiver/Delegates/ERCXXXXReceiverDelegate.sol b/contracts/ProxyReceiver/Delegates/ERCXXXXReceiverDelegate.sol
@@ -15,16 +15,22 @@ contract ERCXXXXReceiverDelegate is ProxyReceiverStorage_002_ERCXXXXFuture {
     bytes4 constant public ERCXXXX_BATCH_RECEIVED = 0x4321BBBB;
     bytes4 constant public NOT_ERCXXXX_RECEIVED = 0xDEADF00D; // Some random value
 
-    function setShouldRejectClash(bool _value) public {
+    function setShouldRejectClash(bool _value) external {
+        require(address(this) == proxy, "Direct call: setShouldRejectClash");
+
         shouldReject = _value;
     }
 
-    function setShouldRejectXXXX(bool _value) public {
+    function setShouldRejectXXXX(bool _value) external {
+        require(address(this) == proxy, "Direct call: setShouldRejectXXXX");
+
         shouldRejectXXXX = _value;
     }
 
     function onERCXXXXReceived(address _operator, address _from, uint256 _id, uint256 _value, bytes calldata _data) external view returns(bytes4) {
-        (_operator); (_from); (_id); (_value); (_data);
+        (_operator); (_from); (_id); (_value); (_data);  // solidity, be quiet please
+
+        require(address(this) == proxy, "Direct call: onERCXXXXReceived");
 
         if (shouldRejectXXXX == true) {
             return NOT_ERCXXXX_RECEIVED;
@@ -34,7 +40,9 @@ contract ERCXXXXReceiverDelegate is ProxyReceiverStorage_002_ERCXXXXFuture {
     }
 
     function onERCXXXXBatchReceived(address _operator, address _from, uint256[] calldata _ids, uint256[] calldata _values, bytes calldata _data) external view returns(bytes4) {
-        (_operator); (_from); (_ids); (_values); (_data);
+        (_operator); (_from); (_ids); (_values); (_data);  // solidity, be quiet please
+
+        require(address(this) == proxy, "Direct call: onERCXXXXBatchReceived");
 
         if (shouldRejectXXXX == true) {
             return NOT_ERCXXXX_RECEIVED;

diff --git a/contracts/ProxyReceiver/ProxyBaseStorage.sol b/contracts/ProxyReceiver/ProxyBaseStorage.sol
@@ -22,6 +22,9 @@ contract ProxyBaseStorage {
     // signature => index+1
     mapping(bytes => uint256) internal funcSignatureToIndex;
 
+    // proxy address of itself, can be used for cross-delegate calls but also safety checking.
+    address proxy;
+
     ///////////////////////////////////////////////////////////////////////////////////////////////
 
 }
diff --git a/contracts/ProxyReceiver/ProxyReceiver.sol b/contracts/ProxyReceiver/ProxyReceiver.sol
@@ -28,13 +28,15 @@ contract ProxyReceiver is ProxyBaseStorage, IERC1538 {
 
     constructor() public {
 
+        proxy = address(this);
+
         //Adding ERC1538 updateContract function
         bytes memory signature = "updateContract(address,string,string)";
         bytes4 funcId = bytes4(keccak256(signature));
-        delegates[funcId] = address(this);
+        delegates[funcId] = proxy;
         funcSignatures.push(signature);
         funcSignatureToIndex[signature] = funcSignatures.length;
-        emit FunctionUpdate(funcId, address(0), address(this), string(signature));
+        emit FunctionUpdate(funcId, address(0), proxy, string(signature));
         emit CommitMessage("Added ERC1538 updateContract function at contract creation");
     }
 

diff --git a/test/ERC1155ProxyTest.js b/test/ERC1155ProxyTest.js
@@ -278,4 +278,8 @@ contract('ERC1155ProxyTest - tests sending 1155 items to an ERC1538 supported pr
         await proxyXXXXDelegate.setShouldRejectXXXX(true);
         await testSafeTransferFrom(user1, user1, receiverContract.address, hammerId, 1, web3.utils.fromAscii('SomethingMeaningfull'), 'testSafeTransferFrom receiver 1155');
     });
+
+    it('attempt direct call to a delegate', async () => {
+        await expectThrow(receiverDelegateERC1155.onERC1155Received(zeroAddress, zeroAddress, 0, 0, web3.utils.fromAscii('')));
+    });
 });