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);
      }
  }
}
}
  • 1
    What address is the contract deployed at? – 0xcaff Aug 8 '17 at 18:55
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 9 '17 at 2: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! – user9402 Aug 9 '17 at 2:23
  • I answered that one. – Thomas Jay Rush Aug 9 '17 at 2: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? – user9402 Aug 9 '17 at 2: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 9 '17 at 2:30

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.