Ovcode Smart Contract Audit

OVCODE Blockchain engaged New Alchemy to perform an audit of the smart contracts involved in the creation and distribution of the OVC token used for their upcoming verification system. The engagement was technical in nature and focused on identifying security issues in the design and implementation of the contracts, finding differences between the contracts' implementation and their behavior as described in public documentation, and finding any other issues with the contracts that may impact their trustworthiness.

Ovcode provided New Alchemy with access to the relevant source code and whitepaper. The audit was performed over the course of one week. This document describes the issues discovered in the audit.

After receiving New Alchemy’s report, Ovcode made several changes to the smart contracts. This document has been updated to reflect those changes.

Files Audited

The code reviewed by New Alchemy is in the GitHub repository https://github.com/OVCODE-Switzerland/ovc-smart-contract/ at commit hash 1c272df7f6fd3900b53301fd8c714f8503e5a548.

After receiving New Alchemy’s audit, Ovcode made changes to their smart contracts. These are reflected at commit hash c80ddd420a3dbbb7efb4c8621dcc130c20770073.

New Alchemy’s audit was additionally guided by Ovcode’s whitepaper. After receiving the initial report, Ovcode released an updated version of the whitepaper and added the OVCOIN whitepaper to GitHub.

New Alchemy’s audit uncovered several findings, including one moderate and multiple minor issues. The audit highlights several areas that could be improved in order to increase the overall security and reliability of the contracts.

Ovcode’s crowdsale process is not explicitly documented, and operates in a manner in which the token purchasers must place a large amount of trust in Ovcode to perform the crowdsale correctly and honestly. While this is likely an oversight, Ovcode should consider formally documenting the crowdsale process in their whitepaper.

Note: During retesting, Ovcode provided access to the OVCOIN whitepaper, which documents this process. The above is left for contextual purposes.

Much of the code is very similar to well-known contracts, such as OpenZeppelin. Since there were subtle differences in how Ovcode implemented this functionality, New Alchemy focused specifically on the discrepancies and on code that is entirely new.

New Alchemy re-tested each of the issues and noted any new or updated functionality in the smart contracts. The most significant change to the contracts is usage of hard-coded start and end times for the crowdsale. This makes the crowdsale a much more transparent process for token purchasers. In addition to this, Ovcode added support for two-phase ownership transfers and made it so unsold OVC tokens can only be transfered after the end of the crowdsale.

General Discussion

While there are minor issues, Ovcode has taken care to implement a clean set of contracts that are well-written. The overall code-base is quite small, which creates a minimal target for attack.

The way the Ovcode contracts are implemented requires that token purchasers place a significant amount of trust in Ovcode. The token sale contract owner has the ability to mint/burn tokens and freeze accounts at will. Ovcode plans to use OVCOIN as a utility token, and

The contracts are slightly out-of-date, using version 0.4.21 (and 0.4.18 for SafeMath.sol) while the current version of Solidity is 0.4.22. This should be updated to the newest version before the contracts are deployed. Ovcode fixed this issue after receiving New Alchemy's report, updating all contracts to 0.4.22.

Moderate Issues

Fixed: Crowdsale process is not transparent

The Ovcode whitepaper does not describe the fine details of the OVC crowdsale. This includes the number of tokens that will be generated, where portions of that token are initially distributed (e.g. private investors, developers), and how long the crowdsale will operate. In addition to this, the contract owner can mint tokens at will, arbitrarily freeze accounts, and start or stop the crowdsale at any time. Combined, these issues create an environment where token purchasers have to place a large amount of trust in Ovcode.

In addition to this, the contract owner can claim any unsold tokens at any point after turning off purchases. It is not clear whether these tokens should be maintained or burned, and the reasoning is not documented. Maintaining tokens after the crowdsale has the potential to dramatically alter the worth of tokens for individuals receiving initial tokens.

It is possible to view this information via comments within the contracts, but Ovcode should document the process more formally.

Re-test results: Ovcode changed the purchase process such that it no longer requires manual activation by the contract owner. In addition, Ovcode has added checks that compare the block timestamp against pre-defined dates. Ovcode provided access to the OVCOIN whitepaper, which outlines dates and token amounts, as well.

Finally, Ovcode changed OVC.sol such that the contract owner can only transfer unsold tokens after the end of the crowdsale.

Minor Issues

Fixed: Lack of two-phase ownership transfer

The Owner.sol contract implements a very simple scheme for contract ownership. The owner can unilaterally transfer ownership to a different address. However, if the owner of a contract makes a mistake in entering the address of an intended new owner, then the contract can become irrecoverably unowned.

In order to preclude this, New Alchemy recommends implementing two-phase ownership transfer. In this model, the original owner designates a new owner, but does not actually transfer ownership. The new owner then accepts ownership and completes the transfer. This can be implemented as follows:

contract Ownable {
    address public owner;
    address public newOwner
    
    event OwnershipTransferred(address indexed oldOwner, address indexed newOwner);
    
    function Ownable() public {
        owner = msg.sender;
        newOwner = address(0);
    }
    
    modifier onlyOwner() {
        require(msg.sender == owner);
        _;
    }
    
    function transferOwnership(address _newOwner) public onlyOwner {
        require(address(0) != _newOwner);
        newOwner = _newOwner;
    }
    
    function acceptOwnership() public {
        require(msg.sender == newOwner);
        OwnershipTransferred(owner, msg.sender);
        owner = msg.sender;
        newOwner = address(0);
    }
}

Re-test results: Ovcode has fixed this issue by integrating the suggested fix into a new ‘Ownable.sol’ contract.

Line by line comments

This section lists comments on design decisions and code quality made by New Alchemy during the review. They are not known to represent security flaws.

OVCLockAllocation.sol

Lines 40, 48, 55, 69:

These locations use the now variable, which is an alias for block.timestamp. Per the Ethereum technical specification, this value can be manipulated by miners up to 900 seconds per block. While unlikely, it is possible that a miner could maliciously alter the behavior of the contract by altering the block timestamp. See https://github.com/ethereum/wiki/wiki/Safety#timestamp-dependence for more information.

Re-test results: This information is intended only for awareness of the issue — the solution will likely cause a more significant deviation than what could be caused by a malicious miner. The caveat to this is that it will be inaccurate for allusers. This is a more “fair” system and may be necessary when launching a crowdsale, for instance if there are incentives that can be reached or restrictions bypassed by purchasing tokens early.

Line 9:

ovc variable should explicitly define visibility.

Re-test results: Ovcode has fixed this issue by declaring the variable public.

OVC.sol

Lines 41–51:

The token amounts for ICO1 and ICO2 seem to be reversed, based on the OVCOIN whitepaper.

Disclaimer

The audit makes no statements or warranties 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.

New Alchemy is a leading blockchain strategy and technology consulting group specializing in tokenization. One of the only companies to offer a full spectrum of guidance from tactical technical execution to high-level theoretical modeling, New Alchemy provides C-level strategy, smart contract development, project management, token design, marketing services, and security audits to the most innovative startups worldwide. Get in touch with us at Hello@NewAlchemy.io