OneSwap Series 11 - Security Verification, Fool-proof Design, and Friction Prevention for ETH Contracts

in #oneswap4 years ago

Introduction

As we all know, the blockchain world follows such a principle: Code is law. Ethereum smart contracts developed on Solidity contain a series of storage states to support the functions of Dapp; during the services provided by Dapp, due to the absence of censorship and decentralization of the blockchain, any organization or individual can call it at will. At this time, to protect the contract to make it operate according to the predetermined logic, it is necessary to stay alert throughout the contract operation to avoid the state of the contract deviating from the predetermined track, which is one source of security hazards.

Such security hazards include not only the notorious coin theft incidents; to avoid security risks, we must at least do the following:

  1. Security verification: avoid abnormal external input or return values ​​of other contracts that cause unexpected situations in the execution of this contract causing capital loss or internal state mess-up
  2. Fool-proof design: honest but careless users may call the contract in a wrong way or call the wrong contract, incurring losses to themselves; a well-designed contract will try to avoid the user's loss by stopping execution upon detecting a user's mistake or ensuring some remedies even after he makes a mistake
  3. Friction: Just like an object is difficult to slide on a surface with a large friction coefficient, a smart contract cannot be executed if it encounters frequent abnormalities and reversions; what's worse, friction will make a smart contract unable to serve users normally. Hackers may take advantage of the friction in a contract for attacks that harm others yet do themselves no good: they attempt to disturb the normal operation of your contract even they gain no benefits by doing so.

This article first introduces the exception-raising keywords provided in the Solidity language, and then, with OneSwap as an example, elaborates on the considerations for security verification, fool-proof design, and friction reduction when they write contracts.

Raising Exceptions in Solidity

Ethereum provides three exception-raising mechanisms to check the parameters received by the contract and some intermediate states generated during the operation of the contract; by doing so, it can avoid a malicious user's input destroying the persistent state inside the contract; it can also suspend contract execution in time when an intermediate state does not meet the predetermined demand to reduce the Gas consumption for the user.

In Ethereum, a contract written in Solidity will revert all modifications by the current call to the contract state upon an exception; moreover, when an exception occurs during a sub-call, it will be passed on to the upper level, and the state change of the upper-level contract will be reverted. Note that there are some exceptions here: Solidity supports inserting low-level call instructions (such as: call, delegatecall, staticcall) when writing contracts, and these low-level call instructions indicate execution results through the return value. When an execution error occurs, the first return value of the instruction is "false", and an exception will not be raised to the upper level. The upper-level calls detect the return value of the assembly instruction to learn the executing status of the call instructions and decide whether to revert the execution state of the upper-level contract.

  • Note: When using low-level instructions call, delegatecall, and staticcall to call an owner account that does not exist, the first return value of the instruction is also "true" (which is caused by the low-level implementation of the EVM); therefore, the inspection for account existence must be performed before the low-level call.

The following are details about the three keywords assert, require, and revert which throw exceptions and their application scenarios.

The keywords require and assert are used to check the expression passed in. An exception is thrown in the format of require(expression, "some reason with error"), assert(expression, "some reason with error") when the expression is false; all state changes of the contract are reverted when an exception occurs because the remaining instructions of the contract cannot be safely executed when the expected result is not obtained; at the same time, to maintain the atomicity of a transaction, the safest operation is to revert all changes in the state to make sure the entire transaction does not have any impact on the data on the chain.

The require expression is used to check the input parameters of the calling contract or the return value after the contract is executed. If the result of the expression is false, require throws an exception and reverts all state changes caused by the current transaction. In the low-level implementation of the Solidity language, the require expression is implemented with the 0xfd instruction (REVERT), which does not consume the user's remaining unused gas.

assert is used to detect errors in the contract and check the execution state of the contract. Under normal circumstances, the execution path of the contract will not trigger the assert exception, but in some scenarios carefully constructed by malicious users, an exception may be triggered as it fails to meet the inspection conditions of assert; as the punishment, assert will consume all remaining Gas in the current transaction.

Contract developers can use require at the entry of external functions to check the input parameters, return values form sub-call, and some other POSSIBLE errors in the contract method, so that they can interrupt execution immediately when it does not meet expectations and reduce Gas consumption for users. Contract developers can use assert for protection against the errors which are IMPOSSIBLE to occur in normal executions, costing the contract-caller all the gas.

The sample code is as follows

contract DemoContract{

    uint const TICKET = 10;
    address pool;

    function addPositive(uint a, uint b) public payable returns(uint){
    
        require(a > 0 && b > 0, "params are not Positive integer");
        
        // error reason is optional
        require(this.balance >= TICKET );

        assert(pool.send(TICKET), "transfer eth failed");
    }

}

revert is another mechanism to trigger an exception in case of an error and shares the same low-level instruction implementation with require; it is mainly used in scenarios where there are many detection expressions and cannot be implemented with a single line of code.

The sample code is as follows

contract DemoRevert{
    uint state; 
    function operation(uint num) public{
        if (num > 0 && num < 10){

            // some operation
            ......
            
            if (num > 3){
                revert("Invalid Calculate")
            }       
        }
    }

}

Security verification

The necessity of security verification is evident. There are so many hackers staring at on-chain contracts, so you need to assume they can call in various possible ways every function of the contract that can be called and try to see if there are vulnerabilities exposed to them. Therefore, the validity verification for input parameters and the verification of the user's transaction amount are both essential and thus common.

For example, after placing a limit order, we need to check the price and the amount of the order:

            require((amount >> 42) == 0, "OneSwap: INVALID_AMOUNT");
            uint32 m = price32 & DecFloat32.MANTISSA_MASK;
            require(DecFloat32.MIN_MANTISSA <= m && m <= DecFloat32.MAX_MANTISSA, "OneSwap: INVALID_PRICE");

With the price and the order amount, we can calculate the number of tokens that the user should transfer to the contract and save it as ctx.remainAmount, and then we need to check the balance to confirm that the user has indeed transferred so many tokens:

    function _checkRemainAmount(Context memory ctx, bool isBuy) private view {
        ctx.reserveChanged = false;
        uint diff;
        if(isBuy) {
            uint balance = _myBalance(ctx.moneyToken);
            require(balance >= ctx.bookedMoney + ctx.reserveMoney, "OneSwap: MONEY_MISMATCH");
            diff = balance - ctx.bookedMoney - ctx.reserveMoney;
            if(ctx.remainAmount < diff) {
                ctx.reserveMoney += (diff - ctx.remainAmount);
                ctx.reserveChanged = true;
            }
        } else {
            uint balance = _myBalance(ctx.stockToken);
            require(balance >= ctx.bookedStock + ctx.reserveStock, "OneSwap: STOCK_MISMATCH");
            diff = balance - ctx.bookedStock - ctx.reserveStock;
            if(ctx.remainAmount < diff) {
                ctx.reserveStock += (diff - ctx.remainAmount);
                ctx.reserveChanged = true;
            }
        }
        require(ctx.remainAmount <= diff, "OneSwap: DEPOSIT_NOT_ENOUGH");
    }

If the user does not transfer enough tokens, an error will be reported and the execution reverted; if the user has transferred too many tokens, the extra tokens will be counted as contract revenue. Note that even if the extra tokens transferred do not cause the loss of funds in the contract, they will still lead to data inconsistency, which also requires treatment.

For another example, in the BuyBack contract, the pair for token swaps is specified by the contract parameters. What if a malicious user specifies a fake pair and steals the funds? In this case, we need to check the authenticity of the pair in the Factory contract:

    function _removeLiquidity(address pair) private {
        (address a, address b) = IOneSwapFactory(factory).getTokensFromPair(pair);
        require(a != address(0) || b != address(0), "OneSwapBuyback: INVALID_PAIR");
        ......
    }

Foolproof design

Similar to various physical tools in the real world (e.g. the two electrodes of the battery to prevent users from misplacement), contracts developed by Solidity also come with fool-proof design, such as preventing users from transferring ETH to a contract account by mistake or refunding the extra funds. Oneswap has many such fool-proof designs.

For example, the token ownership of ONES is transferable, yet not directly transferred from A to B but in two steps: First, A sends a transaction announcing its intention of transferring the ownership to B; then B sends another transaction suggesting that he accepts the ownership, as shown in the following code:

    modifier onlyOwner() {
        require(msg.sender == _owner, "OneSwapToken: MSG_SENDER_IS_NOT_OWNER");
        _;
    }
    modifier onlyNewOwner() {
        require(msg.sender == _newOwner, "OneSwapToken: MSG_SENDER_IS_NOT_NEW_OWNER");
        _;
    }
    function changeOwner(address ownerToSet) public override onlyOwner {
        require(ownerToSet != address(0), "OneSwapToken: INVALID_OWNER_ADDRESS");
        require(ownerToSet != _owner, "OneSwapToken: NEW_OWNER_IS_THE_SAME_AS_CURRENT_OWNER");
        require(ownerToSet != _newOwner, "OneSwapToken: NEW_OWNER_IS_THE_SAME_AS_CURRENT_NEW_OWNER");

        _newOwner = ownerToSet;
    }
    function updateOwner() public override onlyNewOwner {
        _owner = _newOwner;
        emit OwnerChanged(_newOwner);
    }

Why is it designed in this way? When A sends a transaction to transfer the ownership to B, B's address may be one without a private key or an irrelevant contract address. Only when the private key of B's address does exist and the transaction can be sent, the ownership can finally be transferred to B in Step 2. A can also send a transaction announcing the transfer of the ownership to C once a problem is found with B's address.

For another example, in the OneSwapRouter contract, the callback receive() external payable {} that receives ETH by default is removed; by this method, the user can directly transfer ETH to a contract account. If it is deleted, such transfer will fail.

In the OneSwapRouter contract, the payable modifier is added to the necessary external interfaces only to allow the reception of ETH. It prevents users from transferring ETH to a contract account by mistake when calling a contract method, for example: the payable modifier is not added to the liquidity-removing interface of removeLiquidity.

You may ask, in the above example of the checkRemainAmount function, if a user transfers too many tokens, the extra tokens are taken as contract revenue. Is this unfriendly to users who make mistakes? No, because ordinary users call Pair through Router, and Router will help them calculate the amount of tokens for transfer. For example: when placing a buying order, Router first calculates the amount of money a user actually needs to transfer to the pool. Then the user can transfer tokens in the exact amount to the pool. Fool-proof detection is also used to prevent him from accidentally transferring ETH.)

function limitOrder(bool isBuy, address pair, uint prevKey, uint price, uint32 id,
        uint stockAmount, uint deadline) external payable override ensure(deadline) {

        (address stock, address money) = _getTokensFromPair(pair);
        {
            (uint _stockAmount, uint _moneyAmount) = IOneSwapPair(pair).calcStockAndMoney(uint64(stockAmount), uint32(price));
            if (isBuy) {
                if (money != address(0)) { require(msg.value == 0, 'OneSwapRouter: NOT_ENTER_ETH_VALUE'); }
                _safeTransferFrom(money, msg.sender, pair, _moneyAmount);
            } 
            ......
        }
        IOneSwapPair(pair).addLimitOrder(isBuy, msg.sender, uint64(stockAmount), uint32(price), id, uint72(prevKey));
}

Due to the features of the AMM algorithm and the unpredictability of the order of on-chain transactions, every time a user deposits tokens to get liquidity, it is impossible to accurately obtain the ratio of the two assets in the pool at the moment the transaction is put on the chain, resulting in surplus in the transferred tokens in most cases when the user calls the OneSwapRouter contract to deposit tokens (because based on UniSwap's AMM algorithm, the two tokens deposited need to be proportional). At this time, to prevent losses in a user's assets, the OneSwapRouter will automatically transfer the excess assets back to the user.

function _safeTransferFrom(address token, address from, address to, uint value) internal {
    if (token == address(0)) {
        _safeTransferETH(to, value);
        uint inputValue = msg.value;
        if (inputValue > value) { _safeTransferETH(msg.sender, inputValue - value); }
        return;
    }
    .....        
}        

Friction prevention

Not all unreasonable parameters in a contract must throw exceptions using require. Take the simplest example: although the transfer amount of ERC20 tokens is unreasonably 0, it will not have any impact on the contract or the user, so just return without any other actions; too many reverted executions may terminate the calling contract in a surprising way. After all, based on common sense, when the transfer amount of assets is 0, the state will not be changed at all. The "unprovoked termination of the calling contract" here is what we call "friction".

During the contract execution, every call of an external contract may fail and the execution may be reverted. Even simple transfers of ETH or ERC20 may fail. When you transfer ETH to a contract, the code of this contract may be executed and then fail. As for ERC20 transfer failures, one of the most common reasons is that the recipient of the token has been blacklisted.

When a taker trades with a counterparty order, the contract needs to transfer a certain amount of ETH or ERC20 tokens to the owner of the counterparty order. If this transfer fails and the transaction is reverted, then the counterparty order cannot be traded and gets stuck, which also hinders the subsequent orders. In OneSwap, the function used for transfer is like this:

    // safely transfer ERC20 tokens, or ETH (when token==0)
    function _safeTransfer(address token, address to, uint value, address ones) internal {
        if(value==0) {return;}
        if(token==address(0)) {
            // limit gas to 9000 to prevent gastoken attacks
            // solhint-disable-next-line avoid-low-level-calls 
            to.call{value: value, gas: 9000}(new bytes(0)); //we ignore its return value purposely
            return;
        }
        // solhint-disable-next-line avoid-low-level-calls 
        (bool success, bytes memory data) = token.call(abi.encodeWithSelector(_SELECTOR, to, value));
        success = success && (data.length == 0 || abi.decode(data, (bool)));
        if(!success) { // for failsafe
            address onesOwner = IOneSwapToken(ones).owner();
            // solhint-disable-next-line avoid-low-level-calls 
            (success, data) = token.call(abi.encodeWithSelector(_SELECTOR, onesOwner, value));
            require(success && (data.length == 0 || abi.decode(data, (bool))), "OneSwap: TRANSFER_FAILED");
        }
    }

When transferring ETH, ignore the return value of call. If the transfer fails, then the recipient must be a malicious contract account which deliberately constructs some malicious scenarios to block the normal operation of the order book. At this time, for the benefit of other users in the order book, we have no other choice but to leave the malicious hackers bearing the loss; transferring ETH to EOAs (external owner accounts) and normal contracts will not fail. If the transfer of ERC20 tokens fails, the assets originally transferred to the recipient will be transferred to the owner of the ONES token. It is a centralized solution to some extent: if the recipient cannot receive the tokens after being blacklisted, you can have the owner of the OneSwap project take care of these tokens on your behalf for some time and then return these tokens when the recipient is removed from the blacklist. In any case, the order book cannot stop working due to the friction generated during transfer.

For example, in the BuyBack contract of the OneSwap project, a similar concept is also applied to reduce the friction. BuyBack contract extracts the transaction fee from the trading pairs, and if the transaction fee of a trading pairs is 0, it just skips this pair instead of throwing any exception.

function removeLiquidity(address[] calldata pairs) external override {
    for (uint256 i = 0; i < pairs.length; i++) {
        _removeLiquidity(pairs[i]);
    }
}
function _removeLiquidity(address pair) private {
    ....

    uint256 amt = IERC20(pair).balanceOf(address(this));
    if (amt == 0) { return; }
    ....
}

Summary

This article introduces three exception-throwing mechanisms, and use some real cases to describe how to perform security verification using them. Then it also elaborates on some common fool-proof designs that aim to protect users from possible asset losses due to misoperation. Finally, it introduces some examples of reducing call friction to minimize the blocking during contract execution.

Reference:

Error handling: Assert, Require, Revert and Exceptions

Coin Marketplace

STEEM 0.30
TRX 0.12
JST 0.033
BTC 64307.66
ETH 3146.33
USDT 1.00
SBD 3.88