Introduction

CoinFabrik was requested to audit the contracts for the TOSC mission. Firstly, we are going to present a abstract of our discoveries and secondly, we are going to present the small print of our findings.

Abstract

The contracts audited are from the TOSC repository. The mission was initially acquired in a .zip file, and the audit is predicated on the file model specified by the md5 hash taken with the md5sum command, which could be seen within the following record.

The audited contracts are:

  • TokenERC20.sol (e6eeb1efcb0e92b27179bd89733325ec): Customary ERC20 Token implementation.
  • TOSC.sol (af4d4792b539d7b7619ac22d1607f29e): Extends TokenERC20 with lock and freeze functionalities.
  • SafeMath.sol (58592b8e24036b716191bc73904dc53f): Customary library for protected math operations.
  • owned.sol (584853a517d14f659ec9562d23c6a293): Customary contract which permits to set and switch the contract’s possession.

Description

TOSC is an implementation of an ERC-20 Token with a number of further options reminiscent of locking tokens and freezing accounts.

TOSC is a small mission. The entire structure is depicted forward:

Notice: Arrows denote inheritance

Analyses

We checked the code for the next errors and assaults:

  • Misuse of the completely different name strategies
  • Integer overflow errors
  • Division by zero errors
  • Outdated model of Solidity compiler
  • Entrance operating assaults
  • Reentrancy assaults
  • Misuse of block timestamps
  • Softlock denial of service assaults
  • Capabilities with extreme fuel price
  • Lacking or misused perform qualifiers
  • Needlessly advanced code and contract interactions
  • Poor or nonexistent error dealing with
  • Failure to make use of a withdrawal sample
  • Inadequate validation of the enter parameters
  • Incorrect dealing with of cryptographic signatures

Detailed findings

Severity Classification

Safety dangers are labeled as follows:

  • Vital: These are points that we handle to take advantage of. They compromise the system significantly. They should be fastened instantly.
  • Medium: These are doubtlessly exploitable points. Regardless that we didn’t handle to take advantage of them or their impression isn’t clear, they could characterize a safety threat within the close to future. We propose fixing them as quickly as potential.
  • Minor: These points characterize issues which are comparatively small or tough to reap the benefits of however could be exploited together with different points. These sorts of points don’t block deployments in manufacturing environments. They need to be taken into consideration and be fastened when potential.
  • Enhancement: These sorts of findings don’t characterize a safety threat. They’re finest practices that we advise to implement.

This classification is summarized within the following desk:

SEVERITY EXPLOITABLE ROADBLOCK TO BE FIXED
Vital Sure Sure Instantly
Medium Within the close to future Sure As quickly as potential
Minor Unlikely No Finally
Enhancement No No Finally

Points Discovered by Severity

Vital severity

No points have been discovered.

Medium severity

No points have been discovered.

Minor severity

Token ERC20 doesn’t totally implement ERC20 Token Customary

To ensure that a token to be deemed as a legitimate ERC20 Token, it should adjust to the ERC-20 Token Customary. Solely these capabilities explicitly marked as “OPTIONAL” won’t be applied, the remaining should have a correct implementation satisfying the Customary necessities.

On this model, the next non-optional capabilities outlined within the Customary are lacking from TokenERC20.sol:

  • transferFrom
  • approve
  • allowance

In addition to the next occasion:

One other contract would possibly attempt to work together with TokenERC20  anticipating one in every of these capabilities to be applied and fail whereas doing it. We suggest studying the ERC-20 Token Customary (https://eips.ethereum.org/EIPS/eip-20) and ensuring that the token meets all necessities specified within the doc.

Lacking return worth in ERC20 perform

Presently the switch perform in TokenERC20.sol is applied as follows:

This isn’t appropriate as a result of within the ERC-20 Token Customary, the switch perform should return a boolean worth which represents the profitable execution of the perform.

An exterior contract would possibly name switch anticipating a return worth which it is going to lookup in reminiscence, nonetheless since this perform doesn’t return something, the caller will learn rubbish, or worse, delicate information from reminiscence.

frozenAddress in TOSC is public

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

This might be what the programmer meant. Nevertheless, the next accessor perform can also be outlined:

It specifies that solely the proprietor of the contract would possibly question the frozen addresses, nonetheless, this may be bypassed by accessing frozenAddress straight because it’s public.

Both public visibility is meant, subsequently getfrozenAddress is pointless, or solely the proprietor can learn from the mapping, during which case frozenAddress shouldn’t be public.

Enhancements

Outdated compiler model

The mission contract’s make the most of the next pragma for specifying the compiler model: pragma solidity ^0.4.24;
As of the date of this report’s writing the most recent accessible Solidity compiler model is the 0.6.Eight model.

We strongly suggest modifying the contract to make use of the most recent accessible model. Provided that proper now the mission dimension is small it shouldn’t take too lengthy; as soon as the mission grows bigger it is going to require extra work to take action.

In case you resolve to stick with the 0.4.X collection anyway, the next pragma ought to be used to permit solely the most recent model within the talked about collection: pragma solidity 0.4.26

The variety of decimal locations for Tokens ought to be 18

In TokenERC20.sol the decimal locations for the token are set to eight:

It’s strongly really helpful, and customarily agreed amongst Ethereum builders, to choose 18 decimal locations for ERC20 Tokens. It’s because Ether itself makes use of 18 decimal locations, and smaller values may lead to lack of precision when working values and/or interoperability issues with different contracts sooner or later.

There are uncommon events when a smaller worth is likely to be justified, which we assume isn’t the case due to the remark. In case Eight decimal locations had been meant, the rationale should be correctly documented within the code.

Switch of worth Zero doesn’t fireplace Switch occasion

Within the _transfer perform in TokenERC20.sol the next require could be seen:

If _value is the same as 0, then the require fails as a result of the strict inequality isn’t met. Anyway, the ERC20 normal specifies:
Notice Transfers of Zero values MUST be handled as regular transfers and fireplace the Switch occasion.”

That may not occur on this case as a result of the emit assertion is rarely reached. Furthermore, this require isn’t actually essential since in SafeMath.sol the add perform already checks if the result’s larger than or equal to the operand. The identical pointless verify is made in TOSC.sol albeit this time not with a strict inequality.

A number of public capabilities could be declared as exterior

Presently the next capabilities are marked as public and could be declared as exterior as a result of they don’t seem to be referred to as from inside the similar contract:

In TokenERC20.sol:

In TOSC.sol:

  • freezeAddress
  • PercentLock
  • removePercentLock
  • burn
  • transferOwnership

Marking capabilities as exterior tells the compiler the capabilities won’t be referred to as from inside the similar contract, which permits to make optimizations and cut back the fuel price on mentioned capabilities.

Notice that we didn’t point out switch which might be exterior, however because the ERC-20 Specification signifies that it should be declared as public it ought to keep as such.

Keep in mind that in case you ever intend to name this perform from the identical contract, it ought to be modified once more to have public visibility.

Lacking error messages in require statements

The require() statements can take an optionally available parameter to specify the error message which will likely be proven to the consumer when the situation fails to be met. We propose to take action in each assertion potential as to supply the consumer with details about what went mistaken. For instance, in TOSC.sol _transfer perform:

Conclusion

This mission offers a partial implementation of the ERC-20 normal with some further options. The code is easy and practical, and the safety points we discovered could be simply fastened.

We suggest migrating to an up to date model of Solidity since that may present the most recent options, optimizations and bug fixes accessible. 

If ERC-20 compatibility is important we additionally suggest including lacking capabilities and fixing incorrect signatures to make the token ERC-20 compliant.

Disclaimer: This audit report isn’t a safety guarantee, funding recommendation, or an approval of the TOSC mission since CoinFabrik has not reviewed its platform. Furthermore, it doesn’t present a wise contract code faultlessness assure.

LEAVE A REPLY

Please enter your comment!
Please enter your name here