BlockState Smart Contracts Security Analysis

Ivan Ivanitskiy
SmartDec Cybersecurity Blog
7 min readNov 13, 2018

--

In this report, we consider the security of the BlockState project. Our task is to find and describe security issues in the smart contracts of the platform.

Disclaimer

The audit does not give any warranties on the security of the code. One audit cannot be considered enough. We always recommend proceeding with several independent audits and a public bug bounty program to ensure the security of smart contracts. Besides, security audit is not an investment advice.

Summary

In this report, we have considered the security of BlockState smart contract. We have performed our audit according to the procedure described below.

The initial audit has shown two critical issues, as well as many medium and low severity issues. They severely endangered project security. However, all the critical and medium severity issues were fixed in The latest version of the code.

General recommendations

The code is of good quality. Thus, we do not have any additional recommendations.

Procedure

In our audit, we consider the following crucial features of the smart contract code:

1. Whether the code is secure.

2. Whether the code corresponds to the documentation (the developers have not provided the documentation).

3. Whether the code meets ​best​ ​practices​ ​in​ ​efficient​ ​use​ ​of​ ​gas,​ ​code readability, etc.

We perform our audit according to the following procedure:

Automated analysis

  • we scan project’s smart contract with our own Solidity static code analyzer SmartCheck
  • we scan project’s smart contract with several publicly available automated Solidity analysis tools such as Remix and Solhint
  • we manually verify (reject or confirm) all the issues found by tools

Manual audit

  • we manually analyze smart contract for security vulnerabilities
  • we check smart contract logic
  • we check ERC20 compliance

Report

  • we reflect all the gathered information in the report

Project overview

Project description

In our analysis, we consider smart contracts code (“CTF15Token.sol”, sha1sum: f4a194402b4a24b2169426fde54b485c8f790b42).

The latest version of the code

We have performed the check of the fixed vulnerabilities in the latest version of the code (“ctf15token.zip”, sha1sum: 2ed2444440713c87cd6dd88e55cbe76832dc4701) and the documentation (“CTF15Token.md”, sha1sum: af0b5c415b86d4b211b78bdfdc58596bbc978b3b).

Project architecture

For the audit, we have been provided with the smart contract code.

The code successfully compiles with the compiler version 0.4.24

In the latest version of the code, the developers used truffle framework, implemented deploy script and tests.

The provided solidity file contains the following contracts:

· CTF15Token inherits StandardToken, BurnableToken, and Ownable contracts from OpenZeppelin library

The total LOC of audited Solidity sources is 122.

Automated analysis

We used several publicly available automated Solidity analysis tools. Here are the combined results of SmartCheck, Solhint, and Remix

All the issues found by tools were manually checked (rejected or confirmed).

Cases when these issues lead to actual bugs or vulnerabilities are described in the next section.

Manual analysis

The contracts were completely manually analyzed, their logic was checked and compared with the one described in the documentation. Besides, the results of the automated analysis were manually verified. All confirmed issues are described below.

Critical issues

Critical issues seriously endanger smart contracts security. We highly recommend fixing them.

Locked money

There is a payable function in CTF15Token contract:

function GenerateNewTokens(uint ethIndex) payable public returns(bool)

However, there is no way of transferring ETH from the contract. Thus, all the ETH transferred to the contract address will be lost. We highly recommend removing payable keyword or implementing withdrawal functionality.

The issue has been fixed by the developers and is not present in the latest version of the code.

Bug in the code

There is a bug in the code (lines 172–173):

balances[msg.sender] = balances[msg.sender].sub(amount);
_burn(msg.sender, amount);

In the current implementation, the tokens will be subtracted from the msg.sender’s balance twice: firstly, in line 172, secondly, in _burn() function. We recommend removing line 172 from the code.

The issue has been fixed by the developers and is not present in the latest version of the code.

Medium severity issues

Medium issues can influence smart contracts operation in current implementation. We highly recommend addressing them.

No deployment script

There are no deploy scripts. Bugs and vulnerabilities often appear in deployment scripts and severely endanger system’s security.
We highly recommend developing and testing deployment scripts very carefully.

The issue has been fixed by the developers and is not present in the latest version of the code.

No tests

The provided code does not contain tests. Testing is crucial for code security and audit does not replace tests in any way.
We highly recommend both covering the code with tests and making sure that the test coverage is sufficient.

The issue has been fixed by the developers and is not present in the latest version of the code.

Low severity issues

Low severity issues can influence smart contracts operation in future versions of code. We recommend taking them into account.

Compiler version

Solidity source files indicate the versions of the compiler they can be compiled with. Example:

pragma solidity ^0.4.21; // bad: compiles w 0.4.21 and above
pragma solidity 0.4.21; // good: compiles w 0.4.21 only

We recommend following the latter example, as future compiler versions may handle certain language constructions in a way the developers have not foreseen. Besides, we recommend using the latest compiler version — 0.4.24 at the moment.

The issue has been fixed by the developers and is not present in the latest version of the code.

Excessive inheritance

CTF15Token contract inherits StandardToken twice: explicitly and through BurnableToken.

We recommend removing StandardToken from the list of inherited contracts.

The issue has been fixed by the developers and is not present in the latest version of the code.

Deprecated constructions

Deprecated construction is used at lines 133, 144:

function CalculateIndexPrice() public constant returns(uint64)
function getUSCtFromETH(uint256 ethAmount, uint ethIndex) public constant returns(uint256)

We recommend using view instead of constant, since this keyword is deprecated and will not compile since compiler version 0.5.0.

The issue has been fixed by the developers and is not present in the latest version of the code.

Redundant code

The following lines are redundant:

1. lines 28–30

totalSupply_ = INITIAL_SUPPLY;
balances[msg.sender] = INITIAL_SUPPLY;
emit Transfer(0x0, msg.sender, INITIAL_SUPPLY);

As INITIAL_SUPPLY constant is set to zero (line 20), there is no need in either setting zero values to totalSupply_ and balances[msg.sender] or emitting Transfer event.

The issue has been fixed by the developers and is not present in the latest version of the code.

2. lines 111, 186

require(msg.sender == owner);

OnlyOwner modifier is implemented in Ownable contract, which is inherited by CTF15Token, and repeats mentioned lines. This modifier can also be used in GenerateNewTokens() function as it is actually callable only by the owner of the contract.

The issue has been fixed by the developers and is not present in the latest version of the code.

3. line 42

uint64 public buyBackPrice;

The variable can only be changed using SetBuyBackPrice() function. However, the audited code does not depend on its value.

Comment from the developers:

“The “buybackPrice” (and newly added “indexValue”) are to be read from external applications, so their presence is relevant to the contract.”

4. line 41

uint8 public coinNumber;

The variable is actually constant as it never changes. We recommend declaring this variable as constant.

The issue has been fixed by the developers and is not present in the latest version of the code.

5. line 91

balances[_to] = balances[_to].add(_amount);

add() function from SafeMath library was used at the previous line. Moreover, balances[_to] cannot exceed totalSupply_. Hence, there is no chance of overflow here and use of add() function is excessive.

We highly recommend removing redundant code in order to improve code readability and transparency and decrease cost of deployment and execution.

Precision issue

Solidity operates only with integers. Thus, variables multiplication should be done before the division in order to reduce the rounding errors. totalUSCtAmount variable is a return value of getUSCtFromETH() function. The last operation in this function is division. After that, the totalUSCtAmount is multiplied by coinShares[i] (line 161):

buyOrders[i] = (coinShares[i] * totalUSCtAmount) / 1000;

We recommend changing the order of division and multiplication.

The issue has been fixed by the developers and is not present in the latest version of the code.

This audit was performed by SmartDec, a security team specialized in static code analysis, decompilation and secure development.

Feel free to use SmartCheck, our smart contract security tool for Solidity language, and follow us on Medium. We are also available for smart contract development and auditing work.

--

--

Product Manager at PARSIQ. Co-host at Basic Block podcast. Bitcoin, Ethereum, InfoSec. Libertarianism, MMA, IPSC practitioner.