Introduction
This blog post presents the results of a security audit of a smart contract performed by Blaze Information Security and made public on behalf of the client Array.io (formerly known as Annihilat.io). This post contains the very same information and findings present in the report published at the end of December 2017.
The audit was performed by Victor Farias (project lead) and Julio Fort of Blaze Information Security.
We were pleased to learn the team of Array.io takes security seriously and engaged three different companies to audit their smart contracts.
Disclaimer: This document presents the findings of a security review of the smart contracts under the scope of the audit. As a time-boxed and best-effort exercise, it does not guarantee there are no other security issues in the smart contract. The results of this audit should not be read as investment advice.
Reporting
This document presents the results of a Smart Contract Security Review for Array.io. This engagement aimed to verify whether the smart contract only does what it is intended to do and to discover security vulnerabilities that could negatively affect the Array.io ANNI token before the contract gets deployed into the blockchain network.
Array.io is an idea of funding based on the concept of Initial Development Offering (IDO). Essentially, it works as a salary token for contributors to a project that raised capital in an IDO. Array.io uses an Ethereum ERC20-based token and contracts were written in Solidity. Details about Array.io and IDO can be found in the whitepaper.
The analysis focused on vulnerabilities related to implementation and on issues caused by architecture and design errors, as well as inconsistencies between the documentation and the code.
For each code pattern non-compliant with the Ethereum token standard or to the contract specification, deviation of best practices, and vulnerability discovered during the assessment, Blaze Information Security attributed a risk severity rating and, whenever possible, validated the existence of the vulnerability with a working exploit code.
The main objectives of the assessment were the following:
- Identify the main security-related issues present in the smart contract
- Assess the level of secure coding practices present in the project
- Obtain evidence for each vulnerability and, if possible, develop
a working exploit - Report, in a clear and easy-to-reproduce manner, all procedures used to replicate the issue
- Recommend mitigation factors and fixes for each defect identified in the analysis
- Provide context with a real risk scenario based on a realistic threat model
Executive summary
The engagement was performed in a period of five business days, including report writing. The smart contract security review commenced on 12/12/2017 and ended on 18/12/2017, finishing with the preliminary version of this report.
On 21/12/2017 all findings reported by Blaze Information Security were fixed accordingly by Array.io. The issues are no longer present in the code of the contracts and were fixed in commits b287f07393f8b5b67cbde5c1d70dfc31b9cd5aa1 and afdf264d030e8313fd65e1c8c236d2a958eff7c7.
The audit was done with the assistance of automated tools as well as subjected to manual review. The generated EVM code was not inspected in this assessment.
There were three issues discovered in the contracts audited in this engagement. Overall, the contracts under scope contained significant vulnerabilities that could lead to the loss of tokens and cause a significant impact on the operations of Array.io. They also lacked defensive security coding patterns and had other non-recommended Solidity programming practices.
It is important to note these vulnerabilities are no longer present, as they have been corrected by Array.io, and the fixes reviewed by the auditors.
Scope
The scope of this security review is comprised of two smart contracts written in Solidity.
- Project name: annihilatio
- Commit: 8ad0a1bdc7dff7e40ad5cf61aea89deaa982eab5
Token_flat.sol 345 (lines)
Multisig_flat.sol (498 lines)
The code audited is open source and can be found at https://github.com/annihilatio/ido/tree/8ad0a1bdc7dff7e40ad5cf61aea89deaa982eab5/smart-contracts
Smart contract security review
Our security-oriented smart contract review follows an organized methodology with the intent to identify the largest number of vulnerabilities in the contracts under scope from the perspective of a motivated, technically capable, and persistent adversary.
Special attention is directed towards critical areas of the smart contract such as the burning of tokens and the functioning of the multi-signature. Our process also looks into other common implementation issues that lead to problems like reentrancy, mathematical overflows, and underflows, gas-related denial of service, etc.
Blaze’s smart contract review methodology involves automated and manual audit techniques. The applications are subjected to a round of dynamic analysis using tools like linters, program profilers, and source code security scanners.
The contracts have their source code manually inspected for security flaws. This type of analysis has the ability to detect issues that are missed by automated scanners and static analyzers, as it can discover edge cases and business logic-related problems.
Technical summary
Description of the smart contracts
- Multisig wallet
In order to safely store the funds and balance consensus among the owners, array.io uses a multi-signature wallet to control the token. This contract can be used to change the configuration of the token, for example, to set new values for the share that goes to the investor, and founders, and to the project, call TGE to go live, etc.
- Token
This contract is the ANNI token per se. It contains all functions related to minting, transference of tokens among wallets, function to check the balance of a wallet, burning of tokens, and other functionalities outlined in the token specification.
Observations about Multisig_flat.sol
There were numerous errors when trying to run Multisig_flat.sol in automated tools. Blaze Information Security suggests the developers should review the code of the contract to understand why most Solidity security tools could not parse it.
Vulnerabilities
1. Impossibility to trade ANNI tokens back into ETH will hold investor’s funds
Fixed in commit afdf264d030e8313fd65e1c8c236d2a958eff7c7
Severity: Critical
The contract has a function for token holders to convert their ANNI tokens into Ether (ETH). This function, known as burn() is expected to work by first transferring the amount requested in tokens to a “burn address” and subsequently transferring ETH to whoever called the function.
Nevertheless, during the code review Blaze Information Security noticed that the transfer() function does not allow a transfer to a zero address, so the balance of the sender is never updated, and an exception will take place due to a missing condition to satisfy a require(), hence his or hers can call the function to burn tokens multiple times but no action will happen.
From Token_Flat.sig line 135:
address constant public burnAddress = 0×0;line 217:
function burn(uint _amount) public isNotTgeLive
noAnyReentrancy returns(bool _success) {
require(balances[msg.sender] >= _amount);
transfer(burnAddress, amount); // here a transfer is supposed to happen to the burn address
msg.sender.transfer(amount);
Burn(msg.sender, _amount);
return true;
}line 59:
function transfer(address _to, uint value) isNotFrozenOnly
onlyPayloadSize(2 * 32) returns (bool success) {
require(_to != address(0)); // the burn address is 0×0, require() will not be satisfied and will throw
require(value <= balances[msg.sender]); // this line will never be executedGiven the code constructs above, when a burn() function is called it will attempt to execute a transfer like transfer(0×0, _amount) and the statement require(_to != address(0)); will not complete as expected. The impact of this issue is severe, as investors that hold ANNI tokens will never be able to convert their tokens back to ETH.
- Solution
There may be different solutions to this issue. Blaze recommends not calling the transfer function in burn() but instead verifying the amount and deducting it from the account:
require(value <= balances[msg.sender]);
balances[msg.sender] = balances[msg.sender].sub(value);2. Deviation from technical specifications of the contract and code for liquidating tokens
Fixed in commit b287f07393f8b5b67cbde5c1d70dfc31b9cd5aa1
Severity: Medium
According to the specifications of the smart contract (https://github.com/annihilatio/ido/blob/master/SMART-CONTRACT-SPECS.md#burning):
The tokens can be liquidated at any time by a token holder, at this stage tokens are burnt and the token contract sends the same amount of ETH to token holder.
Token holder obviously can not burn more tokens than he owns. Also as tokens are burnt total supply is decreased by the same number of tokens.From Token_flat.sol:
/// @dev Burn tokens to burnAddress from msg.sender wallet
/// @param _amount Amount of tokens
function burn(uint _amount)
public
isNotTgeLive
noAnyReentrancy
returns(bool _success)
{
require(balances[msg.sender] >= _amount);
transfer(burnAddress, amount);
msg.sender.transfer(amount);
Burn(msg.sender, _amount);
return true;
}According to the code above the token liquidation (burn event) cannot be called anytime, as opposed to what the documentation says, but only when TGE (Token Generation Event) is not live.
The auditing team understands this issue does not bring any negative security impact to the contract per se, but it is certainly a deviation from the intended functionality
outlined in the technical specifications of the smart contract.
- Solution
Consider removing the modifier isNotTgeLive from the function burn(). If the code is actually what reflects the business logic, change the documentation to reflect it accurately.
3. Absence of explicit visibility in some function declarations
Fixed in commit b287f07393f8b5b67cbde5c1d70dfc31b9cd5aa1
Severity: Low
The audit revealed that with the exception of two functions, _finishTge() and _mint(uint,uint,uint), both marked as internal, all other functions of the contracts are public as some of them have not been explicitly labeled. Many of these functions are state-changing.
By default Solidity marks as public all non-labeled functions, making them callable by external agents in the network. In order to restrict this behavior, a developer should use the labels internal or private to prevent them from being called from the
outside.
While Blaze Information Security noticed there were different checks in the functions to prevent abuse from external parties calling them, not labeling functions explicitly is considered a bad programming practice and should be avoided. This recommendation hopefully will also provide the development team with an opportunity to review the visibility of the functions and reconsider their current label.
A function profiling of the audited contracts, including the visibility status of each function, can be found in Appendix B of this report
- Reference
https://consensys.github.io/smart-contract-best-practices/recommendations/
- Solution
Add explicit visibility of functions and state variables.
Additional remarks
- At MultiSigWallet contract notNull() modifier could be moved from addTransaction(address destination, uint value, bytes data) to functions submitTransaction(), setLiveTx() and setFinishedTx() to verify it at an earlier stage.
- At MultiSigWallet function isConfirmed() does not explicitly return false.
- At MultiSigWallet in the loop inside getTransactionIds() function, the var ‘i’ could be initialized with the value of variable ‘from’ instead of 0 to save gas.
- At MultiSigWallet could the “IToken token” variable be changed through function “setToken” by any owner without a election? This seems to defeat the purpose of multi-signature and the idea of reaching a consensus in order to perform an action in the wallet.
- Both contracts start with the following code construct:
pragma solidity ^0.4.15;According to the best practices outlined in https://consensys.github.io/smart-contractbest-practices/recommendations/#lock-pragmas-to-specific-compiler-version it should be:
pragma solidity 0.4.15;Conclusion
The ultimate goal of a security assessment is to bring the opportunity to better illustrate the risk of an organization and help make it understand and validate its security posture against potential threats to its business.
With that in mind, Blaze Information Security provides the following recommendations that we believe should be adopted as the next steps to further enhance the security posture of the smart contracts:
- Fix all issues presented in the report and consider the observations made in the remarks section;
- Perform another round of audit to verify the fixes;
- Consider establishing a bug bounty program, as it is becoming increasingly common among companies in the smart contract and blockchain field.
Blaze Information Security would like to thank the team of Annihilat.io for their support and assistance during the entire engagement. We sincerely hope to work with Array.io (Annihilat.io) again in the near future.
