2

Yep, my fault for playing around with contracts on the live net so I'm only to blame but wondered if anyone with more knowledge can help or just confirm that my 1 ETH is now forever stuck or there is a way I can get it.

I know what what the problem is, it's that I've used the safeWithdrawal once already so the var 'amountRaised' has not been reset and if I send more it doesn't match.

What happened was I sent 0.5 ETH, then used safeWithdrawl and got it back fine. Later I sent 1 ETH and tried to safeWithdrawl that. Because amountRaised didn't clear it tries to withdraw 1.5 ETH when there is only 1ETH and would do the same if more was sent to it. Any help or am I stuck?

pragma solidity ^0.4.2;
contract token { function transfer(address receiver, uint amount); }

contract Crowdsale {
    address public beneficiary;
    uint public fundingGoal; uint public amountRaised; uint public 
    deadline; uint public price;
    token public tokenReward;
    mapping(address => uint256) public balanceOf;
    bool fundingGoalReached = false;
    event GoalReached(address beneficiary, uint amountRaised);
    event FundTransfer(address backer, uint amount, bool 
    isContribution);
    bool crowdsaleClosed = false;

/* data structure to hold information about campaign contributors */

/*  at initialization, setup the owner */
function Crowdsale(
    address ifSuccessfulSendTo,
    uint fundingGoalInEthers,
    uint durationInMinutes,
    token addressOfTokenUsedAsReward
) {
    beneficiary = ifSuccessfulSendTo;
    fundingGoal = fundingGoalInEthers * 1 ether;
    deadline = now + durationInMinutes * 1 minutes;
    price = 25;
    tokenReward = token(addressOfTokenUsedAsReward);
}

/* The function without name is the default function that is called whenever anyone sends funds to a contract */
function () payable {
    if (crowdsaleClosed) throw;
    uint amount = msg.value;
    balanceOf[msg.sender] = amount;
    amountRaised += amount;
    tokenReward.transfer(msg.sender, amount * price);
    FundTransfer(msg.sender, amount, true);
}

modifier afterDeadline() { if (now >= deadline) _; }

/* checks if the goal or time limit has been reached and ends the campaign */
function checkGoalReached() afterDeadline {
    fundingGoalReached = true;
    GoalReached(beneficiary, amountRaised);
}


function closeCrowdSale()  {
  if (beneficiary == msg.sender) {
  crowdsaleClosed = true;
  }
}

function safeWithdrawal() {
  if (beneficiary == msg.sender) {
      if (beneficiary.send(amountRaised)) {
          FundTransfer(beneficiary, amountRaised, false);
      }
  }
}
}
Mike Young
  • 55
  • 5

1 Answers1

1

Warning: Don't try any of this, this is more an observation than a full-fledged answer

Some interesting things to note that may be of use later on.

Within function payable (), you have the following lines;

amountRaised += amount;
tokenReward.transfer(msg.sender, amount * price);

The order of operations with those two lines is unorthodox.

amountRaised += amount should come after tokenReward.transfer(msg.sender, amount * price).

Normally I would transfer funds and then reflect the balance in the contract. Just something to note for the future.

Operating under the assumption that you can overflow a uint256 variable to set it equal to 1, if you can increment amountRaised without sending ether to the contract you could overflow it to match the amount you want to withdraw, and call safeWithdrawal().

In general, check out Truffle & TestRPC. In the future, you can easily write tests for contracts. And you can always manually test your contracts on testnets before deploying on the mainnet.

  • 1
    uin256 does not reset to zero if it overflows--it just overflows. 2 + (largest uint256 - 1) == 1. – Thomas Jay Rush Aug 09 '17 at 02:18
  • @ThomasJayRush Ah, I actually wasn't sure at all what the behavior was for that. I opened a new question about it after I posted this answer. Thanks for the input! –  Aug 09 '17 at 02:23
  • I answered that one. – Thomas Jay Rush Aug 09 '17 at 02:25
  • Just pulled it up, +1. So the question is, would overflowing amountRaised (setting amountRaised to 1) still be useful in this case? Also, does that mean that 1 + (largest uint256) == 0? –  Aug 09 '17 at 02:27
  • I don't think 'a malicious user could call the payable function and throw to abort the call halfway through.' First, the caller doesn't throw. The contract does. Second -- no matter who throws, either both happens or neither. A transaction can't change a value and then throw leaving the state half-baked. A throw roles back the entire transaction as if it never happened. You should probably edit this answer. If the OP follows your advice he/she may lose more ether. – Thomas Jay Rush Aug 09 '17 at 02:30
  • I think I made it pretty clear that I was just taking a guess here. But I'm at least learning more about contracts myself. I guess I'll have to think over this some more. I'll add a warning to the top of my answer first though –  Aug 09 '17 at 02:35
  • You should edit the answer to remove the conjecture. Other people may follow the advice here and lose ether. Generally I would say you don't want to guess at answers. – Thomas Jay Rush Aug 09 '17 at 02:38
  • @ThomasJayRush hi, you are wrong about overflow, please read this questionhttps://ethereum.stackexchange.com/questions/7293/is-it-possible-to-overflow-uints – BinGoBinBin Aug 09 '17 at 02:41
  • That loop starts from 0. I think he's saying that before it overflows the gas limit will be hit. If you add (2^256-1) plus 2 it will overflow. – Thomas Jay Rush Aug 09 '17 at 02:49
  • Thanks so much people, that was such a great suggestion thank you, although it seems it can't be done thank you for trying to help :) – Mike Young Aug 09 '17 at 09:43
  • Thanks for putting the warning there. I think that's good. – Thomas Jay Rush Aug 09 '17 at 19:53