Attention! S in Ethereum stands for Security. Part 3. Solidity in practice


We present the third part of the series devoted to typical vulnerabilities, attacks and problem areas inherent in smart contracts in the Solidity language, and the Ethereum platform as a whole. Here we talk about what features Solidity has and what vulnerabilities they can turn into in skilled hands.


In the first part, we discussed the front-running attack, various random number generation algorithms and network resiliency with the Proof-of-Authority consensus. The second talked about Integer overflow, ABI encoding / decoding, Uninitialized storage pointer, Type Confusion and how to make a backdoor. And in this part, we will discuss several distinguishing features of Solidity and look at the logical vulnerabilities that can occur in contracts.


The evolution of ether


To begin with, how smart contracts exchange values ​​and user addresses with each other. At the beginning, ethers were transmitted by calling another contract:


msg.sender.call.value(42) // или вот так msg.sender.call.value(42)()

However, when calling a contract without specifying a signature, its fallback function will be called, in which there may be arbitrary code. Such an unusual logic of work led to the famous reentrancy, with the help of which TheDAO was hacked .


After that, a function appeared send , which is also just syntactic sugar - under the hood, it has the same call, only the amount of gas is limited, so reentrancy will not work.


msg.sender.send(42) // msg.sender.call.value(42).gas(2300)() - намного лучше, правда?

However, if something goes wrong and the broadcast cannot be sent, send will not interrupt the execution flow. This behavior can also be critical. For example, the broadcast was not sent, and the status of the contract has already been changed. Someone will be left without ethers .


Therefore, transfer appeared, and it will throw an exception if something goes wrong.


msg.sender.transfer(42) // if (!msg.sender.send(42)) revert()

But she is not a silver bullet. Imagine that we have an array of addresses to which the broadcast should be sent, and if used transfer , the success of the whole operation will depend on each of the recipients - if one does not receive the broadcast, then all changes will be completely rolled back.


And the last moment with the broadcasting is the function selfdestruct .


selfdestruct(where)

In fact, this is a function for destroying a contract, but all the air that remains on the contract will be sent to the address indicated as an argument. And this cannot be avoided in any way - the ether will leave even if the receiving address is a contract, and it does not have a payable fallback function payable (fallback is simply not called). The air will be sent even to the contract that has not yet been created !


Inheritance


In Solidity, to allow multiple inheritance, the C3 linearization algorithm is used (the same as in Python, for example). And for those who were lucky not to step on the rake of multiple inheritance, the final graph will most likely seem unobvious. Consider an example:


contract Grandfather {
    bool public grandfatherCalled;

    function pickUpFromKindergarten() internal {
        grandfatherCalled = true;
    }
}

contract Mom is Grandfather {
    bool public momCalled;

    function pickUpFromKindergarten() internal {
        momCalled = true;
    }
}

contract Dad is Grandfather {
    bool public dadCalled;

    function pickUpFromKindergarten() internal { 
        dadCalled = true;
        super.pickUpFromKindergarten(); 
    }
}

contract Son is Mom, Dad {

    function sonWannaHome() public {
        super.pickUpFromKindergarten();
    }
}

Continue the call graph starting at Son.sonWannaHome ().


Answer

Will be called Dad, and then Mom. Total inheritance is as follows.
Son -> Dad -> Mom -> Grandfather


An example of a more or less plausible contract with a bug regarding multiple inheritance was presented at the Underhanded Solidity Coding Contest .


brain teaser


Smart contracts are written by people, and people often make mistakes ... in the names of variables, constructors ; they forget to restrict access to certain functions (as, for example, in Parity Multisig ), etc. The developer should also carefully monitor the possible onset of the race condition, since any function of the smart contract can be called from any address, at any time. He himself must implement the necessary synchronization primitives and access modifiers so that the smart contract can control the order of the call. In addition, there are things that no code analyzer can find - domain errors. Therefore, this section will discuss copyright vulnerabilities.


Implicit math


In the vast majority of contracts that need to work with mathematics, for example, calculating how many tokens a user will receive for a broadcast, the SafeMath library is used . However, the name can be misleading - in fact, SafeMath only cares about overflows . We offer to consider the following piece of the contract:


contract Crowdsale is Ownable {
    using SafeMath for uint;

    Token public token;
    address public beneficiary;

    uint public collectedWei;
    uint public tokensSold;

    uint public tokensForSale = 7000000000 * 1 ether;
    uint public priceTokenWei = 1 ether / 200;

    bool public crowdsaleFinished = false;

    function purchase() payable {
        require(!crowdsaleFinished);
        require(tokensSold < tokensForSale);
        require(msg.value >= 0.001 ether);

        uint sum = msg.value;
        uint amount = sum.div(priceTokenWei).mul(1 ether); 
        uint retSum = 0;

        if(tokensSold.add(amount) > tokensForSale) {
            uint retAmount = tokensSold.add(amount).sub(tokensForSale);
            retSum = retAmount.mul(priceTokenWei).div(1 ether);

            amount = amount.sub(retAmount);
            sum = sum.sub(retSum);
        }

        tokensSold = tokensSold.add(amount);
        collectedWei = collectedWei.add(sum);

        beneficiary.transfer(sum);
        token.mint(msg.sender, amount);

        if(retSum > 0) {
            msg.sender.transfer(retSum);
        }

        LogNewContribution(msg.sender, amount, sum);
    }
}

Have you noticed anything suspicious? Most likely not, and this is absolutely normal. Let's get it right. Pay attention to the expression sum.div(priceTokenWei).mul(1 ether) - from the point of view of logic, everything is very smooth: "To get the amount of tokens that an investor needs to accrue, you need to divide the amount of ethers by an expression that reflects the price of the token in ether units, and then multiply by 1 ether to bring to the desired units. "


$amount = (sum/5000000000000000) * 1000000000000000000$


But there is a nuance. Each library call (and there are two of them) will receive two uint and return also uint, and this, in turn, means that the fractional part of the first operation will be completely legitimately discarded.


// функция деления из библиотеки SafeMath
function div(uint256 a, uint256 b) internal pure returns (uint256) {
    uint256 c = a / b;
    return c;
}

Thus, by sending not an integer number of ethers to this crowdsale contract, the investor will lose tokens, and the ICO can collect more than expected: D The full contract can be found in solidity_tricks .


Multiple Voting Through Circular Mining History Manipulation


Such a long name hides a funny vulnerability discovered during the audit of PoA network contracts. According to the rules of the network, there are 12 or more validators in it that can hold various votes, including changing the key (and, accordingly, address) of the validator. In order for the validator to not be able to change the key and vote twice, the smart contract keeps a history of all keys. And when validating the vote, it checks that among the voters there is no his ancestor.


So, every time the key changes, it is placed in mapping, where it refers to the previous key. Therefore, with each new change, the contract has the opportunity to go through the history of keys. However, in this configuration, without additional checks, the validator can loop the key history and thereby truncate the old keys:


1) The validator with key A registers the vote X, then requests a change of key. After that, he has key B in his hands. If he tries to vote with his new key right now, he will fail, because key A is in history B:
History(B): B => A => 0x


2) Therefore, the validator requests a key change again, receives the key C. Again, right now the trick will not work for the same reason:
History(C): C => B => A => 0x


3) Then the validator requests a change of the key C to the key B. After that, the history of the keys is looped between B and C, and does not contain A:
History(B): B => C => B => C => B => ...


The validator can now use key B or C to vote in the X vote a second time. Fix and original report , as well as other vulnerabilities.


Right now, you may reasonably have two questions:


  • Why can a validator change its key so easily?
    Answer: Actually, not just, but through voting . However, there is no verification that the key was already there (at least, the authors of the report thought so).
  • Why in theory does the infinite sequence obtained in step 3 not generate an infinite loop during verification (which should lead to an out-of-gas exception)?
    Answer: take a look at the check
function
function areOldMiningKeysVoted(uint256 _id, address _miningKey) public view returns(bool) {
    VotingData storage ballot = votingState[_id];
    IKeysManager keysManager = IKeysManager(getKeysManager());
    for (uint8 i = 0; i < maxOldMiningKeysDeepCheck; i++) {
        address oldMiningKey = keysManager.miningKeyHistory(_miningKey);
        if (oldMiningKey == address(0)) {
            return false;
        }
        if (ballot.voters[oldMiningKey]) {
            return true;
        } else {
            _miningKey = oldMiningKey;
        }
    }
    return false;
}

In any case, the loop size will be no more than 256 repetitions due to the fact that the variable i is defined as uint8.


The real possibility of exploiting this vulnerability raises questions from the author, however, it will still be useful to those who are going to store a unidirectional list in mapping after they find out in the chat on stackoverflow that arrays are expensive :)


Generous refund


The following vulnerability is rather related to ignorance / misunderstanding of the values ​​of global variables. We offer you to take a look at one of the possible implementations of the commit-reveal scheme :


pragma solidity ^0.4.4;

import 'common/Object.sol';
import 'token/Recipient.sol';

/**
 * @title Random number generator contract
 */
contract Random is Object, Recipient {
    struct Seed {
        bytes32 seed;
        uint256 entropy;
        uint256 blockNum;
    }

    /**
     * @dev Random seed data
     */
    Seed[] public randomSeed;

    /**
     * @dev Get length of random seed data
     */
    function randomSeedLength() constant returns (uint256)
    { return randomSeed.length; }

    /**
     * @dev Minimal count of seed data parts
     */
    uint256 public minEntropy;

    /**
     * @dev Set minimal count of seed data
     * @param _entropy Count of seed data parts
     */
    function setMinEntropy(uint256 _entropy) onlyOwner
    { minEntropy = _entropy; }

    /**
     * @dev Put new seed data part
     * @param _hash Random hash
     */
    function put(bytes32 _hash) {
        if (randomSeed.length == 0)
            randomSeed.push(Seed("", 0, 0));

        var latest = randomSeed[randomSeed.length - 1];

        if (latest.entropy < minEntropy) {
            latest.seed = sha3(latest.seed, _hash);
            latest.entropy += 1;
            latest.blockNum = block.number;
        } else {
            randomSeed.push(Seed(_hash, 1, block.number));
        }

        // Refund transaction gas cost
        if (!msg.sender.send(msg.gas * tx.gasprice)) throw;
    }

    /**
     * @dev Get random number
     * @param _id Seed ident
     * @param _range Random number range value
     */
    function get(uint256 _id, uint256 _range) constant returns (uint256) {
        var seed = randomSeed[_id];

        if (seed.entropy < minEntropy) throw;

        return uint256(seed.seed) % _range;
    }
}

Did you notice that the smart contract returns the spent gas when committing the next part of the seed (see the put function)? In itself, the desire to return the spent commission does not fit into the paradigm of the Ethereum platform, but this is not the worst. The vulnerability here is that the msg.gas value is controlled by the sender and means the remaining gas. Thus, the attacker, by manipulating the gas of the transaction and its price, can withdraw all funds from the contract.


Instead of a conclusion


In this article, we examined only a few logical vulnerabilities in order to form the reader’s intuition about places where you can make mistakes when writing smart contracts. In fact, there are most of all such logical (copyright) vulnerabilities in contracts. They are connected, first of all, with business logic or subject area. This also suggests that most vulnerabilities in contracts cannot be detected automatically, at least until they begin to allow the user to describe the criteria for "improper behavior". By the way, in the next part we will consider what tools do exist and what they are good for in their current state.


PS I express my gratitude Raz0r for an example of a Generous refund :)