Solidity General Secure Coding Guidelines-web3资讯-ODAILY

1. Variable storage/assignment/deletion:

secondary title

1. Do not store any sensitive information on the chain

Vulnerability details:Blockchain-based transparency,Any contract data deployed on the chain is transparent and visible

, even for private modified variables. Because the visibility of private is only for functions and external contracts, any user can obtain these values by retrieving data on the chain. In this case, any confidentiality operation that is expected to be guaranteed by on-chain private modification is insecure.

contract Eocene{
    mapping(address => bytes 32) candidate;
    uint private seed = 0x 12341 3d;
    function select() public{
            bytes 32 result = keccak 256(abi.encodePacked(seed));
            if(result == candidate[msg.sender]){
                payable(msg.sender).transfer( 1 ether);
            }
        }
}

If the following simple sweepstakes code exists:

Even if the seed has been declared as a private variable, anyone can retrieve the seed value on the chain through the contract address and the slot position of the seed, so as to calculate the corresponding value to obtain eth.

Do not store any key values for verification in the contract, store the key values off-chain, and only implement the corresponding verification logic on the chain.

secondary title

2. Pay attention to variable default values

Vulnerability details:

In solidity, the initial value of a variable is 0/false. In this case, if the influence of the variables initial value is not considered when making a judgment based on a certain variable, it may cause corresponding security problems.

contract Eocene{
    mapping(address => bool) unlocked;
    uint averageDrop;
    address token;
    function setAverageDrop() public {
        averageDrop = 1000;
    }
    function drop() public {
        if(unlocked[msg.sender] == false){
            ERC 20(token).transfer(msg.sender, averageDrop);
        }
    }
}

Consider the following airdrop unlock code:

The original intention of the contract is to distribute tokens to all unlocked addresses, but it ignores that in solidity, the initial value of all variables is 0/false. In the mapping type, the key is only used for splicing with the slot, and the address corresponding to the key in the storage is calculated through keccak 256, which means that any address, whether initialized or not, will have a storage corresponding to it, and the initial value Usually 0/false.

Do not make critical judgments based on the default value of variables under any circumstances, especially in variables based on mapping types, and strictly prevent such problems.

secondary title

3. Delete the value of struct type that is no longer used

Vulnerability details:

For any mapping type, when the value field type is struct and the corresponding value is no longer needed, the delete setting should be used to delete the value. Otherwise, the value will still remain in the corresponding slot.

contract Eocene{
    struct Stake{
        uint amount;
        uint needReceive;
        uint startTime;
    }
    mapping(address => Stake) stakes;
    mapping(address => bool) staker;
    function getStake() public{
        Stake memory _stake = stakes[msg.sender];
        (msg.sender).transfer(_stake.needReceive);
        staker[msg.sender] = false;
        // delete stakes[msg.sender] // need do but dont
    }
    function calReceive() public{
        require(staker[msg.sender],not staker);
        stakes[msg.sender].needReceive = stakes[msg.sender].amount * (block.time - stakes[msg.sender].startTime);
        stakes[msg.sender].amount = 0;
    }
}

Consider code of the form:

The contract code above calculates the amount of eth obtained based on the pledge amount and pledge time, but after the pledge is completed, only the value of staker[msg.sender] is set to false, and the corresponding stakes[msg.sender] still exists. So the attacker can call the getStake() function without restriction to get eth.

Repair measures:Of course, the above code also has some other auxiliary problems that lead to the existence of vulnerabilities, but you should be aware that,, otherwise the value pair always exists in the corresponding slot.

2. Function definition:

secondary title

1. Visibility of declared functions that must be shown

Vulnerability details:The default visibility of functions is public,For any function, its visibility must be declared explicitly

, to prevent the existence of loopholes caused by negligence, especially when the function is nested in multiple layers to call the underlying function, to prevent the underlying function from being incorrectly given visibility due to negligence.

contract Eocene{
    mapping(address => bool) whitelist;
    function _a() {
        payable(msg.sender).transfer( 1 ether);
    }
    function a() public{
        require(whitelist[msg.sender],not in whitelist);
        _a();
    }
}

Consider the following example of vulnerable code:

The a() function limits the whitelist address through require, and transfers money to the corresponding address after passing. Under normal circumstances, _a() should not be called externally, but here because the visibility of _a() is not explicitly declared, it is regarded as As public, it can be directly called by the outside.

Explicitly declare the visibility of all functions, especially for functions that cannot be directly called from the outside, they must be explicitly declared as protect or private.

secondary title

2. Function reentrancy attack

Vulnerability details:

As with any function, you must consider the problems that may arise after reentrancy. Reentrancy here includes all reentrancy problems caused by external calls such as transfer/send/call/staticall.

contract Fund {
    mapping(address => uint) shares;
    function withdraw() public {
    if (payable(msg.sender).send(shares[msg.sender]))
    shares[msg.sender] = 0;
    }
}

Consider the following code form:

For the appeal contract, when msg.sender is malicious, it can cause msg.sender to extract the balance of all current contracts without limit. But we must also be aware that when calling transfer/send/call/staticall and any external contract functions, it may cause reentrancy problems.

According to the specific implementation details of the contract, the implementation of key variables can be modified first. For example, in the appeal contract, the value of shares[msg.sender] can be recorded first, and then the send operation can be performed after setting shares[msg.sender] to 0. Of course, it can also be realized through the combination of global variables and decorators.

3. External interaction

secondary title

1. Limit the address and function list of external calls

Vulnerability details:

For calls to external functions, under reasonable circumstances, the contract address and contract function to be called must be limited

contract Eocene{
    function callExt(address _target, bytes calldata data) public{
        _target.call(data);
    }
    function delegateCallExt(address _target, bytes calldata data) public{
        _target.delegatecall(data);
    }
}

Consider the following code form:

The function callExt is used to call any address of any function. In this case, it is easy to cause re-entry problems. Once the contract has any Token assets in any wallet, it can directly call the transfer function of the corresponding Token through this function. Walk.

However, if there is no restriction when calling the delegateCallExt function, it may cause the contract to be destructed directly, resulting in the transfer of the entire contract address balance and the contract being destroyed.

For calls to any external contract, first consider whether the address can be restricted by the whitelist, and further consider whether the function name of the specified address can be restricted.

secondary title

2. When using call, send, delegatecall, staticcall, the judgment of external calls should not only depend on exceptions, but also judge by return value

Vulnerability details:

The appeal function does not cause revert due to an internal error, but just returns revert. Any time they are used, the return value of the function must be used to determine whether the execution was successful.

contract Eocene{
    address token; //any token address
    function deposit(uint amount) public{
        token.call(abi.EncodeWithSignature("transferfrom(address from, address receipt, uint amount)"), msg.sender, address(this), amount);
        mint(msg.sender, amount);
    }
}

Consider the following sample code:

In the deposit() function, the contract first tries to transfer the specified token of msg.sender to the current address. After the transfer is successful, it mints some current coins for msg.sender. However, since the .call function will not revert the entire transaction when it fails, even if it fails to transfer any currency from msg.sender to the current address, it will still mint the current currency of amount to msg.sender.

The judgment of the execution result of call, send, delegatecall, and staticcall must be based on its return value, not on whether it reverts.

Fourth, access control:

secondary title

1. No identity authentication based on tx.origin

Vulnerability details:Do not authenticate based on tx.origin

, tx.origin is the initiator of the entire transaction and will not change with the recursive call of the contract. Any authentication based on tx.origin cannot guarantee that tx.origin is msg.sender. Its tx.origin-based authentication also increases user account security.

contract Eocene{
    mapping(address=>bool) whitelist;
    function freeDeposit() public{
        require(whitelist[tx.origin],not in whitelist);
        payable(msg.sender).transfer( 1 ether);
    }
}

Consider the following example of a vulnerability:

When any address in the whitelist is induced by some phishing link to call any seemingly harmless malicious contract address and function, and the malicious address calls the freeDeposit function of the sample code, the assets that should belong to the whitelist address Will be transferred to the malicious contract address.

Repair measures:No authentication based on tx.origin. Or for the appeal code, when changed to payable(tx.origin).transfer( 1 ether), it will not cause problems., but use require(whitelist[msg.sender],not in whitelist); to make a judgment.

secondary title

2. Do not judge eos accounts based on the return value of extcodesize

Vulnerability details:In the initialization phase of the contract code, even if the address is a contract address, the return value of extcodesize will be 0

, if the judgment is made based on the return value, the result obtained is inaccurate.

contract Eocene{
    function withdraw() public{
        uint size;
        assembly {
        size := extcodesize(caller())
        }
        require(size== 0,"not eos account");
        msg.sender.transfer( 1 ether);
    }
}

Consider the following code form:

In the withdraw function of the appeal contract, it is hoped that only the EOS account can obtain tokens through the extcodesize return value, but it ignores that when the contract is initialized, the extcodesize return value for the contract address is also 0. As a result, the judgment is inaccurate, and any address can obtain tokens from the contract.

Do not make judgments based on whether the external address is the contract address at any time, and try to ensure that the contract code functions normally under any type of account.

5. Arithmetic operations

secondary title

1. When any numerical operation, consider the overflow problem

Vulnerability details:

The overflow problem refers to the overflow problem caused when the contract does integer operations. The main reason is that any numeric type has its maximum length. When the operation of two integers exceeds its maximum value, the excess part will be truncated, causing problems.

contract Eocene{
    mapping(address=>uint) balanceof;
    function withdraw(uint amount) public{
        payable(msg.sender).transfer(amount);
        balanceof[msg.sender] = balanceof[msg.sender]-amount;
        require(balanceof[msg.sender] >= 0,not enough balance);
    }
}

Consider the following code form:

For the appeal function, consider that when balanceof[msg.sender] < amount, because the balanceof type is limited to unsigned integer, the total calculation result will result in a negative value of int type, and when converted to uint type, it will be a very large positive value , at this time, the restriction of require is bypassed, and the attacker can steal any number of tokens from the contract summary.

Repair measures:, to ensure that the final calculation result does not cause overflow.

secondary title

2. When doing any integer operations, use the int type carefully

Vulnerability details:

When doing any calculations with integer types, be careful to convert uint type to int type for calculations, unless you need this kind of operation. Because when converting a uint type integer to an int type, some cases of overflow for the uint type will be invalid in the int type.

contract Eocene{
    int public result;
    uint public uresult;
    function cal(uint _a, uint _b) public{
        result = int(_a)-int(_b);
        uresult = uint(result);
    }
}

Consider the following code form:

When compiling with solidity version 0.8.0 or later, if you call `cal( 0, 1)`, even though ` 0-1` causes overflow in uint, it will not cause overflow in the calculation of int type revert (because the result of 0-1 is in the range of type int). And when the result value is converted to uint type, it is the result value after the actual uint type calculation overflows, which leads to the overflow problem in disguise.

But it should be noted that if cal(type(int).min, type(int).max) is called here, revert will still be triggered, because the integer calculation at this time is also beyond the range of the int type

Repair measures:in progress. If the integer operation itself needs to overflow, consider using uncheck to wrap the uint type operation implementation.

secondary title

3. In any operation that may lose precision, prevent unacceptable loss of precision by extending

Vulnerability details:

When doing any integer operation, the possible problems caused by loss of precision are considered, and its precision is extended.

contract Eocene {
    uint totalsupply;
    mapping(address=>uint) balancesof;
    uint BasePrice = 1 e 16;
    function mint() public payable {
        uint tokens = msg.value/BasePrice;
        balancesof[msg.sender] += tokens;
        totalsupply += tokens;
    }
}

Consider the following code of the form:

Considering the appeal contract, the number of tokens that should be obtained is calculated by msg.value/basePrice in mint, but due to the accuracy of `/` calculation, all parts when msg.value is less than 1 e 16 will be locked in the contract In the process, this will not only lead to a waste of eth, but also quite bad for the user experience.

For integer calculations that may have missing precision, first expand the integer by `* 1 eN` (N is the required precision size).

6. Random value:

secondary title

1. Do not use any guessable/operated on-chain data as a random number seed

Vulnerability details:

Due to the particularity of the blockchain, there is no truly random value on the chain, and any data on the chain should not be used as a random value or random number seed. Consider obtaining random values from off-chain.

contract Eocene{
    function winner(bytes 32 value) public payable{
        require(msg.value > 0.5 ether,"not enough value");
        if(value == keccak 256(abi.encodePacked(block.timestamp))){
        msg.sender.transfer( 1 ether);
        }
    }
}

The code example is as follows:

For the appeal contract, use the current block time tag to calculate the random value, compare it with the random value submitted by the user, and reward the user with the same random value. It seems to be a random situation based on time, but in fact any user who uses keccak 256 (abi.encodePacked(block.timestamp)) can calculate the value through the contract call and send it to the contract code of the winner function to get the eth . In addition, we should also understand that the value of block.timestamp can be maliciously tampered by miners, and it is not necessarily fair.

Repair measures:, consider using chainlink to obtain offline random values

Seven, DOS:

secondary title

1. Any operation that copies the entire state variable array to a memory variable is prohibited

Vulnerability details:

Soliditys limit on the available memory size of the function is much lower than storage (0x ffffffffffffffff), any behavior of copying the dynamic array as a whole to memory may exceed the available memory size, resulting in revert.

contract Eocene{
    uint[] id;
    function pop(uint amount) public{
        require(amount>0,not valid amount);
        uint[] memory _id=id; // this may be revert because of memory space limit
            for(uint i= 0;i0,not valid amount);
        id.push(amount);
    }
}

Consider the following code form:

In the above code, `uint[] memory _id=id;` will put the variable value of `uint[] id;` in the storage into the memory, and the push function can insert values into `uint[] id;`, and because Solidity has restrictions on memory space. Once the length of `uint[] id;` exceeds `(0x ffffffffffffffff-0x 40)/0x 20-1`, it will cause excessive memory usage and revert. It means that the pop function of this contract can never be executed successfully, or any function with `uint[] memory _id=id;` operation cannot be executed successfully.

Repair measures:Do not copy variable dynamic arrays to memory at any timeAny function whose memory usage exceeds this value cannot execute successfully。

secondary title

2. In any for loop, loop judgment cannot be based on external modifiable variables

Vulnerability details:

If the judgment of any for loop is based on external modifiable variables, there may be a problem that the external modifiable variables are too large and the gas consumption is too high. DOS attacks occur when gas consumption is high enough for each contract caller to bear.

contract Eocene{
    uint[] id;
    function pop(uint amount) public{
        require(amount>0,not valid amount);
        for(uint i= 0;i0,not valid amount);
        id.push(amount);
    }
}

Consider the following code:

Here we delete the operation of copying the array from storage to memory, but another problem with this code is that the for loop is based on the length of `uint[] id;`, and the length of id can only be increased but not decreased in the contract, which means The gas consumed by the pop() function will become larger and larger. When the gas is too large to exceed the maximum gas consumption that can be tolerated by executing the pop function, few people will execute the pop, and a DOS attack will be realized.

Repair measures:, any loop operation should be able to judge the maximum length of its execution to prevent the existence of dos problems.

secondary title

3. In the loop, use try/catch to catch undetermined exceptions

Vulnerability details:

contract Eocene{
    address[] candidates;
    mapping(address=>uint) balanceof;
    function claim() public{
        for(uint i= 0;i0,no balance);
            payable(candidate).transfer(balanceof[candidate]);
        }
    }
}

If there is a revert that may be caused by an external address inside any loop, the capture of the revert must be considered. Otherwise, once any inner loop execution fails, all previous gas consumption will be meaningless. However, when the execution failure inside the loop can be controlled by the external address, if there is no try/catch to catch possible exceptions, it may cause the loop judgment to never be completed completely, realizing a DOS attack.

The code uses a for loop to transfer money to each candidate, but it does not take into account that when any candidate is directly reverted in the fallback or receive function, the loop will never be executed successfully, realizing a DOS attack.

In any for loop, if there is an external call and it is impossible to judge whether the call will revert, you must use try/catch to try to replenish the exception to prevent DOS attacks caused by revert。

8. Use a higher version compiler:

secondary title

1. Use a compiler above 0.8.17 to compile the contract

first level title

- SOL-2022-7

- SOL-2022-6

- Solidity

about Us

At Eocene Research, we provide the insights of intentions and security behind everything you know or dont know of blockchain, and empower every individual and organization to answer complex questions we hadnt even dreamed of back then.

Learn more: [Website] | [Medium] | [Twitter]