CBOND Smart Contract Audit

Introduction

CoinFabrik was asked to audit the contracts for the CBOND project. First we will provide a summary of our discoveries and then we will show the details of our findings.

Summary

The contracts audited are from the CBOND project, provided by the development team in a compressed file.

The project allows users to earn interest by staking cryptobonds (which are non-fungible tokens) between the SYNC ERC-20 Token (implemented in the project) and Uniswap liquidity.

Contracts

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:

CBOND.sol (8708bd9a93daad946f2d42d9a6adee67)
Fairreleaseschedule.sol (fc1fdd6df3273542f0ef54302b5b021d)
Sync.sol (a46067aaa0c0edf76061bf1f1e6fdecc)
priceOracle.sol (fa58f7c588cdb96b583a67a49e41a83c)
manualOracle.sol (b8c9f827c74057f4800dad2daabb5dd2)
AddressStrings.sol (07a3aad1b4d8817d9e2f3c3b9e34de98)
SquareRoot.sol (ee27c0f9851d39eb863586294a0472e9)
IOracle.sol (afc65f68eee6849f3f797a93e1fe5370)

Analyses

The following analyses were performed:

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 of critical severity were found

Medium severity

No issues of medium severity were found

Minor severity

No issues of minor severity were found

Non-security issues

Lucky numbers are not determined properly

A bonus interest is intended to be rewarded when a CBOND ID ends in ‘77’ or ‘777’. The corresponding code is the following:

if(tokenId.sub(tokenId.div(100))==77){
    return LUCKY_EXTRAS[1];
}
if(tokenId.sub(tokenId.div(1000))==777){
    return LUCKY_EXTRAS[2];
}

However, the division and subtraction does not successfully extract the latest 2 and 3 digits, for example, if tokenID equals 1977:

1977-(1977/100)=1957.23 instead of the expected 77.

This mistake means that the lucky bonus will never be awarded when intended, the modulus operation is supposed to be used instead.

Another issue with the code is that it currently uses LUCKY_EXTRAS[1] and LUCKY_EXTRAS[2] as bonus values, yet, when looking at the declaration of LUCKY_EXTRAS we see the following:

Therefore the only valid indexes to access this array are 0 and 1, the current code will report an invalid number for the first bonus and go out of bounds for the second one.

uint256[] public LUCKY_EXTRAS=[500,1000];

The corrected code with both fixes would be:

if(luckyEnabled){
      if(tokenId.mod(100)==77){
        return LUCKY_EXTRAS[0];
      }
      if(tokenId.mod(1000)==777){
        return LUCKY_EXTRAS[1];
      }
}

This issue was fixed by the development team in CBOND.sol (91b652ecbb06690fe75cd1d97616c984) and is now considered fixed.

Enhancements

Function can be pure

The getLiquidityPairIncentiveRate function in CBOND.sol can be marked as pure instead of view, which would save a small amount of gas.

CBOND contract is growing large

Currently some tools report CBOND.sol as being too large and having the possibility to run into problems while deploying it. These warnings are most of the time false-positives. However, refactoring CBOND.sol into two or more smaller contracts will avoid problems with this in the future, as well as increase code readability and maintenance.

Unused public fields in CBOND contract

At the beginning of CBOND.sol the following arrays are declared:

uint256[] public DURATION_MODIFIERS=[825,1650,3300,6600,10000];
uint256[] public DURATION_CALC_LOOPS=[0,0,3,7,11];
mapping(uint256 => uint256) public INDEX_BY_DURATION;

Nevertheless, they are never used anywhere in the code, INDEX_BY_DURATION is assigned in the constructor but it is not used after that either.

Consider removing them if they are unnecessary and do not need to be queried from outside the contract, in order to save some gas.

FRS contract does not use SafeMath

Several mathematical operations can be seen in fairreleaseschedule.sol where direct math operators are used instead of the SafeMath equivalent functions.

This is not marked as a security issue because we could not detect any possible overflows or underflows, and they are probably unlikely since most values in this file depend on the eth sent by the user and would be costly for an attacker to try to overflow a variable.

Nevertheless, we recommend to always use SafeMath to be extra sure that no unintended behaviour is possible, unless for gas-optimization reasons it is decided to not use SafeMath.

Unused variable in matureCBOND function

In the matureCBOND function (CBOND.sol) the beforeMint variable is assigned but never used again:

//provide user with their Sync tokens and their initially deposited liquidity tokens
uint256 beforeMint=syncToken.balanceOf(msg.sender);

We recommend making sure that this function was not intended to be used elsewhere, and in such case removing it in order to avoid further confusions.

Conclusion

Overall the project does not seem to present security issues, however some small mistakes were found that could have been avoided by paying more attention to the code, writing more tests and using static analysis tools. The team was helpful with the documentation provided and they were quick to provide the fixes to the items presented in this document.

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

El post CBOND Smart Contract Audit apareció primero en CoinFabrik Blog.

Text source: CoinFabrik Blog

Smart Contract Audit