Block Consilium Audit Sample

BITSIFU TOKEN CONTRACT AUDIT

Done in August, 2018 by Block Consilium

Introduction

Bitsifu asked us to review their upcoming token contracts. Block Consilium reviewed the system from a technical perspective looking for bugs in their codebase. Overall the code is very well written. Read more Below.

In this Smart Contract audit we’ll cover the following topics:

  1. Disclaimer
  2. Overview of the audit and nice features
  3. Attacks made to the contract
  4. Critical vulnerabilities found in the contract
  5. Medium vulnerabilities found in the contract
  6. Low severity vulnerabilities found
  7. Line by line comments
  8. Summary of the audit

1. Disclaimer

The audit makes no statements or warrantees about utility of the code, safety of the code, suitability of the business model, regulatory regime for the business model, or any other statements about fitness of the contracts to purpose, or their bug free status. The audit documentation is for discussion purposes only.

2. Overview

The project has one Solidity file for both Token and Crowdsale smart contracts, the ERC20CrowdsaleToken.sol file which contains 361 lines of Solidity code. All the functions and state variables are well commented using the natspec documentation for the functions which is good to understand quickly how everything is supposed to work.

Nice Features
The contract provides a good suite of functionality that will be useful for the entire contract AND It USES SafeMath library to check for overflows and underflows which protects against underflow and overflow attacks, All the ERC20 functions are included it's a valid ERC20 token and in addition has some extra functionality for Burning And Crowdsale.

3. Attacks made to the contract

In order to check for the security of the contract, we tested several attacks in order to make sure that the contract is secure and follows best practices.

  • Over and under flows An overflow happens when the limit of the type variable uint256 , 2 ** 256, is exceeded. What happens is that the value resets to zero instead of incrementing more.

    For instance, if I want to assign a value to a uint bigger than 2 ** 256 it will simple go to 0 — this is dangerous.

    On the other hand, an underflow happens when you try to subtract 0 minus a number bigger than 0.For example, if you subtract 0 - 1 the result will be = 2 ** 256 instead of -1.

    This is quite dangerous. This contract DOES check for overflows and underflows by using OpenZeppelin's SafeMath, and there are no instances of direct arithmetic operations which might be dangerous. So this contract is NOT vulnerable to Overflow and Underflow bugs.

  • Replay attack The replay attack consists on making a transaction on one blockchain like the original Ethereum’s blockchain and then repeating it on another blockchain like the Ethereum’s classic blockchain. The ether is transferred like a normal transaction from a blockchain to another. Though it's no longer a problem because since the version 1.5.3 of Geth and 1.4.4 of Parity both implement the attack protection EIP 155 by Vitalik Buterin
    So the people that will use the contract depend on their own ability to be updated with those programs to keep themselves secure.

  • Short address attack This attack affects ERC20 tokens, was discovered by the Golem team and consists of the following:

    A user creates an Ethereum wallet with a trailing 0, which is not hard because it’s only a digit. For instance: 0xiofa8d97756as7df5sd8f75g8675ds8gsdg0

    Then he buys tokens by removing the last zero:
    Buy 1000 tokens from account 0xiofa8d97756as7df5sd8f75g8675ds8gsdg

    If the token contract has enough amount of tokens and the buy function doesn’t check the length of the address of the sender, the Ethereum’s virtual machine will just add zeroes to the transaction until the address is complete.

    The virtual machine will return 256000 for each 1000 tokens bought. This is a bug of the virtual machine that’s yet not fixed so whenever you want to buy tokens make sure to check the length of the address.

    Here is a fix for short address attacks


    modifier onlyPayloadSize(uint size) {  
        assert(msg.data.length >= size + 4);  
        _;  
    }  
    function transfer(address _to, uint256 _value) onlyPayloadSize(2 * 32) {  
        // do stuff  
    }  

    The virtual machine will return 256000 for each 1000 tokens bought. This is a bug of the virtual machine that’s yet not fixed so whenever you want to buy tokens make sure to check the length of the address.

    This contract implements an onlyPayloadSize(uint numwords) modifier for transfer, transferFrom, approve, increaseApproval, and decreaseApproval functions which checks the length of the data before doing token transfers, so this contract is NOT vulnerable to short address attacks.

    You can read more about the attack here: ERC20 Short Address Attacks.

  • Approval Doublespend
    Imagine that Alice approves Mallory to spend 100 tokens. Later, Alice decides to approve Mallory to spend 150 tokens instead. If Mallory is monitoring pending transactions, then when he sees Alice’s new approval he can attempt to quickly spend 100 tokens, racing to get his transaction mined in before Alice’s new approval arrives. If his transaction beats Alice’s, then he can spend another 150 tokens after Alice’s transaction goes through.

    This issue is a consequence of the ERC20 standard, which specifies that approve() takes a replacement value but no prior value. Preventing the attack while complying with ERC20 involves some compromise: users should set the approval to zero, make sure Mallory hasn’t snuck in a spend, then set the new value. In general, this sort of attack is possible with functions which do not encode enough prior state; in this case Alice’s baseline belief of Mallory’s outstanding spent token balance from the Mallory allowance.

    It’s possible for approve() to enforce this behavior without API changes in the ERC20 specification:

    if ((_value != 0) && (approved[msg.sender][_spender] != 0)) return false;

    However, this is just an attempt to modify user behavior. If the user does attempt to change from one non-zero value to another, then the doublespend can still happen, since the attacker will set the value to zero.

    If desired, a nonstandard function can be added to minimize hassle for users. The issue can be fixed with minimal inconvenience by taking a change value rather than a replacement value:


    function increaseApproval (address _spender, uint256 _addedValue)
    onlyPayloadSize(2)
    returns (bool success) {
      uint oldValue = approved[msg.sender][_spender];
      approved[msg.sender][_spender] = safeAdd(oldValue, _addedValue);
      return true;
    }

    Even if this function is added, it’s important to keep the original for compatibility with the ERC20 specification.

    Likely impact of this bug is low for most situations. This contract implements an increaseApproval and a decreaseApproval function, both of which takes the change in value instead of taking the new value, which is really nice.

    For more, see this discussion on github: https://github.com/ethereum/EIPs/issues/20#issuecomment263524729

4. Critical vulnerabilities found in the contract

There aren’t critical issues in the smart contract audited.

5. Medium vulnerabilities found in the contract

There are no Medium vulnerabilities in the contract.

6. Low severity vulnerabilities found

There are no low severity vulnerabilities in the contract.

7. Line by line comments

  • Line 1:
    You’re specifying a pragma version with the caret symbol (^) up front which tells the compiler to use any version of solidity bigger than 0.4.24.
    That's completely fine when you deploy the contract as it's already compiled, however to keep the code unbroken by future compilers just in case some backwards incompatible change is made, it is recommended to save the code without the caret symbol (^) for future use so that it forces to compile only with the version which was the latest or most stable when the code was written.

  • Lines 3 to 31:
    SafeMath library is included for safe arithmetic operations.

  • Lines 34 to 73:
    The Ownable contract makes the contract creator the owner of the contract, so that in ERC20CrowdsaleToken contract the owner is able to set price, start and stop sale, etc, and also the ether which is sent to contract address for buying tokens during crowdsale goes to the owner of the contract immediately.

  • Lines 75 to 224:
    These lines implement the Standard ERC20 token features, along with the onlyPayloadSize(uint numwords) modifier to prevent short address attacks and with increaseApproval and decreaseApproval functions to prevent Approval Doublespend attack.

  • Lines 227 to 251:
    It implements a BurnableToken contract which means the token holders can burn tokens from their balance and thus decrease the total supply of the token, in general when the supply goes down and demand is either same or increases, the value of the tokens increases rapidly.

  • Lines 253 to 268:
    The Token contract now concludes the Burnable and Ownable ERC20 token part, it assigns the name BITSIFU COIN, the symbol BSF, decimals 18, and initialSupply 50000000 (50 Million), to the token, and the constructor sends all the minted initial supply to the contract creator (owner).

  • Lines 270 to 361:
    The ERC20CrowdsaleToken contract implements the crowdsale functionality to the smart contract. It allows the owner to start sale (startSale()), stop sale (stopSale()), set the price of tokens in terms of ether (setPrice(uint)), implements a fallback function (function() payable) for people to buy tokens by sending ether to contract address, implements a forwardFunds() function to transfer incoming ether to owner, and a withdrawTokens(uint) function for the owner to withdraw any unsold tokens from the smart contract.

  • Other nice features:
    The code uses new constructor syntax by using constructor keywords instead of same function name as contracts, and there are no instances of the deprecated throw keyword, it uses new assert(condition), revert(), require(condition) functions which is a nice thing to save gas. This contract is using the new syntax for emitting events with the emit prefix, which is again another nice thing.

8. Summary of the audit

This code is very clean, thoughtfully written and in general well architected. The code conforms closely to the documentation and specification – we loved reading it.

The code is based on OpenZeppelin in many cases. In general, OpenZeppelin’s codebase is good, and this is a relatively safe start.

Overall the code is well commented and clear on what it’s supposed to do for each function. The visibility and state mutability of all the functions are clearly specified, there are no confusions. This is a secure contract that will work as expected after reading the code.

45

Audits Completed

We've completed 45 Flawless Audits, many of which are public and you can find them on our GitHub.

43,786

Lines of Code

Our engineers have carefully reviewed and Audited so many lines of code, sharpening their already awesome skills!

157

Issues Discovered

By catching the vulnerabilities in their code and suggesting fixes, we've probably saved Millions of USD to our customers!

Ready to Work Together?

Request a quote and let's get to work! (audits@blockconsilium.com)