2

Imagine a simple contract that collects Eth. Is there any way to manipulate this such that after users send funds, this.balance =/= max_val? Can this be gamed in a way that will allow isFinished() to never be able to get past the require statement?

uint max_val = 3 * 10** 18; // Can send up to 3 ETH
bool finished = false; // Not finished

// Function to send ETH
function sendEth() payable {
    require(!finished); // Cannot be over
    require(this.balance <= max_val);  // Contribution must be less than or equal to contribution max
}

// Function to finish sending
function isFinished() {
    require(!finished); // Cannot be finished. Cannot be called twice.
    require(this.balance == max_val);  // Must have collected max ETH
    finished = true;
}
2

A couple of comments first:

  1. Why is the function that receives ether called sendEth shouldn't it be called receiveEth because that's what it does?
  2. Why not use modifiers which make you code significantly easier to understand? Easy to understand code == less buggy code.

My guess is that balance is updated after the function, but I'm not sure (see comment below).

pragma solidity ^0.4.11;

contract Testing {

    uint max_val = 3 * 10 ** 18;
    bool finished = false;

    modifier notFinished() {
       if (finished)
           revert();
       _;
    }

    // NOTE: Not sure if this.balance is already updated with
    // the new msg.value (my guess is no). I will edit this code
    // if someone corrects me.
    function receiveEth() notFinished() payable {

       // If the tx would go over max, revert
       if (msg.value + this.balance > max_val)
           revert();

       // If we will have exactly max, we're done
       if (msg.value + this.balance == max_val)
           finished = true;

       // Otherwise, accept the eth
    }
}
1

Judging from your code, it's trivial to send more than max_val because you don't check if msg.value is more than max_value - this.balance. You need to add require(msg.value <= max_value - this.balance) inside sendEth().

  • It is my understanding that right when sendEth() is called, this.balance gets updated, prior to going through any lines of the function. If this is the case, then wouldn't require(msg.value <= max_value - this.balance) be redundant? – blockchaindotsol Aug 3 '17 at 20:19
  • I believe that balance is updated afterward, but I can't find that in docs. – scriptin Aug 3 '17 at 21:43

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.