TOSC Smart Contract Audit

Introduction

CoinFabrik was asked to audit the contracts for the TOSC project. Firstly, we will provide a summary of our discoveries and secondly, we will show the details of our findings.

Table of Contents

Summary

The contracts audited are from the TOSC repository. The project was originally received in a .zip file, and the audit is based on the file version specified by the md5 hash taken with the md5sum command, which can be seen in the following list.

The audited contracts are:

TokenERC20.sol (e6eeb1efcb0e92b27179bd89733325ec): Standard ERC20 Token implementation.
TOSC.sol (af4d4792b539d7b7619ac22d1607f29e): Extends TokenERC20 with lock and freeze functionalities.
SafeMath.sol (58592b8e24036b716191bc73904dc53f): Standard library for safe math operations.
owned.sol (584853a517d14f659ec9562d23c6a293): Standard contract which allows to set and transfer the contract’s ownership.

Description

TOSC is an implementation of an ERC-20 Token with a few additional features such as locking tokens and freezing accounts.

TOSC is a small project. The complete architecture is depicted ahead:

Analyses

We checked the code for the following errors and attacks:

Misuse of the different call methods
Integer overflow errors
Division by zero errors
Outdated version of Solidity compiler
Front running attacks
Reentrancy attacks
Misuse of block timestamps
Softlock denial of service attacks
Functions with excessive gas cost
Missing or misused function qualifiers
Needlessly complex code and contract interactions
Poor or nonexistent error handling
Failure to use a withdrawal pattern
Insufficient validation of the input parameters
Incorrect handling of cryptographic signatures

Detailed findings

Severity Classification

Security risks are classified as follows:

Critical: These are issues that we manage to exploit. They compromise the system seriously. They must be fixed immediately.
Medium: These are potentially exploitable issues. Even though we did not manage to exploit them or their impact is not clear, they might represent a security risk in the near future. We suggest fixing them as soon as possible.
Minor: These issues represent problems that are relatively small or difficult to take advantage of but can be exploited in combination with other issues. These kinds of issues do not block deployments in production environments. They should be taken into account and be fixed when possible.
Enhancement: These kinds of findings do not represent a security risk. They are best practices that we suggest to implement.

This classification is summarized in the following table:

SEVERITY	EXPLOITABLE	ROADBLOCK	TO BE FIXED
Critical	Yes	Yes	Immediately
Medium	In the near future	Yes	As soon as possible
Minor	Unlikely	No	Eventually
Enhancement	No	No	Eventually

Issues Found by Severity

Critical severity

No issues have been found.

Medium severity

No issues have been found.

Minor severity

Token ERC20 does not fully implement ERC20 Token Standard

In order for a token to be deemed as a valid ERC20 Token, it must comply with the ERC-20 Token Standard. Only those functions explicitly marked as “OPTIONAL” might not be implemented, the rest must have a proper implementation satisfying the Standard requirements.

In this version, the following non-optional functions defined in the Standard are missing from TokenERC20.sol:

transferFrom
approve
allowance

As well as the following event:

Approval

Another contract might try to interact with TokenERC20 expecting one of these functions to be implemented and fail while doing it. We recommend reading the ERC-20 Token Standard (https://eips.ethereum.org/EIPS/eip-20) and making sure that the token meets all requirements specified in the document.

Missing return value in ERC20 function

Currently the transfer function in TokenERC20.sol is implemented as follows:

function transfer(address _to, uint256 _value) public {
    _transfer(msg.sender, _to, _value);
}

This is not correct because in the ERC-20 Token Standard, the transfer function must return a boolean value which represents the successful execution of the function.

An external contract might call transfer expecting a return value which it will look up in memory, however since this function does not return anything, the caller will read garbage, or worse, sensitive data from memory.

frozenAddress in TOSC is public

Currently the mapping frozenAddress in TOSC.sol has public visibility:

mapping (address => bool) public frozenAddress;

This could be what the programmer intended. However, the following accessor function is also defined:

function getfrozenAddress(address addr) onlyOwner external view returns(bool){
    return frozenAddress[addr];
}

It specifies that only the owner of the contract might query the frozen addresses, nevertheless, this can be bypassed by accessing frozenAddress directly since it’s public.

Either public visibility is intended, therefore getfrozenAddress is unnecessary, or only the owner can read from the mapping, in which case frozenAddress should not be public.

Enhancements

Outdated compiler version

The project contract’s utilize the following pragma for specifying the compiler version: pragma solidity ^0.4.24;
As of the date of this report’s writing the latest available Solidity compiler version is the 0.6.8 version.

We strongly recommend modifying the contract to use the latest available version. Given that right now the project size is small it should not take too long; once the project grows larger it will require more work to do so.

If you decide to stay with the 0.4.X series anyway, the following pragma should be used to allow only the latest version in the mentioned series: pragma solidity 0.4.26

The number of decimal places for Tokens should be 18

In TokenERC20.sol the decimal places for the token are set to 8:

uint8 public decimals = 8;
// 18 decimals is the strongly suggested default, avoid changing it

It is strongly recommended, and generally agreed among Ethereum developers, to settle on 18 decimal places for ERC20 Tokens. This is because Ether itself uses 18 decimal places, and smaller values could result in loss of precision when operating values and/or interoperability problems with other contracts in the future.

There are rare occasions when a smaller value might be justified, which we assume is not the case because of the comment. In case 8 decimal places were intended, the reason must be properly documented in the code.

Transfer of value 0 does not fire Transfer event

In the _transfer function in TokenERC20.sol the following require can be seen:

require(balanceOf[_to].add(_value) > balanceOf[_to]);

If _value is equal to 0, then the require fails because the strict inequality is not met. Anyway, the ERC20 standard specifies:
“Note Transfers of 0 values MUST be treated as normal transfers and fire the Transfer event.”

That can not happen in this case because the emit statement is never reached. Moreover, this require is not really necessary since in SafeMath.sol the add function already checks if the result is bigger than or equal to the operand. The same unnecessary check is made in TOSC.sol albeit this time not with a strict inequality.

Several public functions can be declared as external

Currently the following functions are marked as public and can be declared as external because they are not called from within the same contract:

In TokenERC20.sol:

burn

In TOSC.sol:

freezeAddress
PercentLock
removePercentLock
burn
transferOwnership

Marking functions as external tells the compiler the functions will not be called from within the same contract, which allows to make optimizations and reduce the gas cost on said functions.

Note that we did not mention transfer which could be external, but since the ERC-20 Specification indicates that it must be declared as public it should stay as such.

Remember that if you ever intend to call this function from the same contract, it should be changed again to have public visibility.

Missing error messages in require statements

The require() statements can take an optional parameter to specify the error message which will be shown to the user when the condition fails to be met. We suggest to do so in every statement possible as to provide the user with information about what went wrong. For example, in TOSC.sol _transfer function:

require(!frozenAddress[_from], "Sender is frozen");
require(!frozenAddress[_to], "Recipient is frozen");

Conclusion

This project provides a partial implementation of the ERC-20 standard with some additional features. The code is simple and functional, and the security issues we found can be easily fixed.

We recommend migrating to an updated version of Solidity since that will provide the latest features, optimizations and bug fixes available.

If ERC-20 compatibility is essential we also recommend adding missing functions and fixing incorrect signatures to make the token ERC-20 compliant.

Disclaimer: This audit report is not a security warranty, investment advice, or an approval of the TOSC project since CoinFabrik has not reviewed its platform. Moreover, it does not provide a smart contract code faultlessness guarantee.