Commit includes

- Reentrancy evaluation and prevention mechanisms in GatewayComposer

.travis.yml

@@ -5,6 +5,7 @@ branches:
   only:
   - master
   - develop
+  - /^release-.*/
 notifications:
   email:
     recipients:

contracts/GatewayComposer.sol

@@ -56,6 +56,9 @@ contract GatewayComposer {
 
     mapping (bytes32 => StakeRequest) public stakeRequests;
 
+    /* Mutex lock status. */
+    bool private isMutexLock;
+
 
     /* Modifiers */
 
@@ -68,6 +71,15 @@ contract GatewayComposer {
         _;
     }
 
+    /** Checks that mutex lock is acquired or not. */
+    modifier isMutexLockAcquired() {
+        require(
+            isMutexLock == false,
+            "Mutex lock is already acquired."
+        );
+        _;
+    }
+
 
     /* Special Functions */
 
@@ -78,6 +90,9 @@ contract GatewayComposer {
      *          - owner address should not be zero
      *          - ValueToken address should not be zero
      *          - BrandedToken address should not be zero
+     *          - owner address should not be equal to valueToken
+     *          - owner address should not be equal to brandedToken
+     *          - ValueToken address should be equal to BrandedToken.valueToken()
      *
      * @param _owner Address of the staker on the value chain.
      * @param _valueToken EIP20Token which is staked.
@@ -102,6 +117,14 @@ contract GatewayComposer {
             address(_brandedToken) != address(0),
             "BrandedToken address is zero."
         );
+        require(
+            _owner != address(_valueToken),
+            "ValueToken address is same as owner address."
+        );
+        require(
+            _owner != address(_brandedToken),
+            "BrandedToken address is same as owner address."
+        );
         require(
             address(_valueToken) == address(_brandedToken.valueToken()),
             "ValueToken should match BrandedToken.valueToken."
@@ -124,6 +147,7 @@ contract GatewayComposer {
      *          - stakeVT can't be 0
      *          - mintBT amount and converted stakeVT amount should be equal
      *          - gateway address can't be zero
+     *          - owner address should not be equal to gateway address
      *          - beneficiary address can't be zero
      *          - successful execution of ValueToken transfer
      *          - successful execution of ValueToken approve
@@ -160,8 +184,11 @@ contract GatewayComposer {
     )
         external
         onlyOwner
+        isMutexLockAcquired
         returns (bytes32 stakeRequestHash_)
     {
+        acquireLock();
+
         require(
             _stakeVT > uint256(0),
             "Stake amount is zero."
@@ -174,6 +201,10 @@ contract GatewayComposer {
             _gateway != address(0),
             "Gateway address is zero."
         );
+        require(
+            owner != _gateway,
+            "Gateway address is same as owner address."
+        );
         require(
             _beneficiary != address(0),
             "Beneficiary address is zero."
@@ -197,6 +228,8 @@ contract GatewayComposer {
             gasLimit: _gasLimit,
             nonce: _nonce
         });
+
+        releaseLock();
     }
 
     /**
@@ -233,7 +266,10 @@ contract GatewayComposer {
         external
         returns (bytes32 messageHash_)
     {
-        StakeRequest storage stakeRequest = stakeRequests[_stakeRequestHash];
+        StakeRequest memory stakeRequest = stakeRequests[_stakeRequestHash];
+
+        delete stakeRequests[_stakeRequestHash];
+
         require(
             stakeRequest.stakeVT > uint256(0),
             "Stake request not found."
@@ -276,8 +312,6 @@ contract GatewayComposer {
             stakeRequest.nonce,
             _hashLock
         );
-
-        delete stakeRequests[_stakeRequestHash];
     }
 
     /**
@@ -305,7 +339,10 @@ contract GatewayComposer {
         onlyOwner
         returns (bool success_)
     {
-        StakeRequest storage stakeRequest = stakeRequests[_stakeRequestHash];
+        StakeRequest memory stakeRequest = stakeRequests[_stakeRequestHash];
+
+        delete stakeRequests[_stakeRequestHash];
+
         require(
             stakeRequest.stakeVT > uint256(0),
             "Stake request not found."
@@ -319,8 +356,6 @@ contract GatewayComposer {
             "ValueToken transfer returned false."
         );
 
-        delete stakeRequests[_stakeRequestHash];
-
         success_ = true;
     }
 
@@ -435,6 +470,7 @@ contract GatewayComposer {
      * @dev Function requires:
      *          - msg.sender is owner
      *          - gateway address can't be zero
+     *          - Gateway address should not be equal to owner address
      *          - successful execution of ValueToken transferFrom
      *          - successful execution of ValueToken approve
      *
@@ -462,6 +498,10 @@ contract GatewayComposer {
             _gateway != address(0),
             "Gateway address is zero."
         );
+        require(
+            owner != _gateway,
+            "Gateway address is same as owner address."
+        );
         require(
             valueToken.transferFrom(msg.sender, address(this), _penalty),
             "ValueToken transferFrom returned false."
@@ -475,4 +515,24 @@ contract GatewayComposer {
 
         return true;
     }
+
+    /* Private Functions */
+
+    /**
+     * @notice This acquires mutex lock.
+     */
+    function acquireLock()
+        private
+    {
+        isMutexLock = true;
+    }
+
+    /**
+     * @notice This releases the mutex lock.
+     */
+    function releaseLock()
+        private
+    {
+        isMutexLock = false;
+    }
 }

test/gateway_composer/constructor.js

@@ -72,6 +72,28 @@ contract('GatewayComposer::constructor', async (accounts) => {
       'BrandedToken address is zero.');
     });
 
+    it('Reverts if ValueToken address is same as owner address.', async () => {
+      await utils.expectRevert(GatewayComposer.new(
+        valueToken,
+        valueToken,
+        brandedToken,
+        { from: deployer },
+      ),
+      'It should revert as valueToken address is same as owner address.',
+      'ValueToken address is same as owner address.');
+    });
+
+    it('Reverts if BrandedToken address is same as owner address.', async () => {
+      await utils.expectRevert(GatewayComposer.new(
+        brandedToken,
+        valueToken,
+        brandedToken,
+        { from: deployer },
+      ),
+      'It should revert as BrandedToken address is same as owner address.',
+      'BrandedToken address is same as owner address.');
+    });
+
     it('Reverts if ValueToken is not equal to BrandedToken.valueToken.', async () => {
       brandedToken = await BrandedToken.new(
         accountProvider.get(),

test/gateway_composer/request_stake.js

@@ -165,6 +165,37 @@ contract('GatewayComposer::requestStake', async (accounts) => {
       'Gateway address is zero.');
     });
 
+    it('Fails when gateway address is same as owner address.', async () => {
+      const {
+        valueToken,
+        gatewayComposer,
+        owner,
+        brandedToken,
+      } = await gatewayComposerUtils.setupGatewayComposer(accountProvider);
+
+      const {
+        stakeAmount,
+      } = await gatewayComposerUtils.approveGatewayComposer(
+        valueToken,
+        gatewayComposer,
+        owner,
+      );
+
+      const mintAmount = await brandedToken.convertToBrandedTokens(stakeAmount);
+      await utils.expectRevert(gatewayComposer.requestStake(
+        stakeAmount,
+        mintAmount,
+        owner,
+        beneficiary,
+        gasPrice,
+        gasLimit,
+        nonce,
+        { from: owner },
+      ),
+      'Should revert because gateway address and owner address are same.',
+      'Gateway address is same as owner address.');
+    });
+
     it('Fails when beneficiary address is zero.', async () => {
       const {
         valueToken,

test/gateway_composer/revert_stake.js

@@ -73,6 +73,27 @@ contract('GatewayComposer::revertStake', async (accounts) => {
       );
     });
 
+    it('Fails when gateway address is same as owner address', async () => {
+      const {
+        gatewayComposer,
+        owner,
+      } = await gatewayComposerUtils.setupGatewayComposer(accountProvider);
+
+      const penalty = 0;
+      const messageHash = web3.utils.soliditySha3('hash');
+
+      await utils.expectRevert(
+        gatewayComposer.revertStake(
+          owner,
+          penalty,
+          messageHash,
+          { from: owner },
+        ),
+        'Should revert as gateway address and owner address are same.',
+        'Gateway address is same as owner address.',
+      );
+    });
+
     it('Fails if valueToken transferFrom returns false', async () => {
       const {
         gatewayComposer,