yuriy77k commented 5 years ago

Audit request

ERC-20 implementation, Presale token management (creating records of presales and allowing users to claim those records at a rate of 1% per day), Crowdsale contract - EOS model with 2 additions: 1) a hidden hard cap, and 2) dynamic daily reserve price (with the ability to rebase the ETH price daily to have an accurate reserve

We have now completed internal testing, please run the audit on commit f162c021c0d3d4c72c2144c80968d5190086300a of the master branch at https://github.com/tubiex/smart-contracts/tree/master/contracts

Here is some documentation to help the audit if needed

ERC-20

Follows OpenZeppelin Standards

Uses the Roles design pattern - https://github.com/OpenZeppelin/openzeppelin-solidity/tree/master/contracts/access/roles

We have a Minter role who mints initial supply and then revoke self

We have a Recoverer who can recover mis-sent ERC20 tokens (anytime for ERC20-contract, for Presale/Bounty/Airdrop contracts TBN tokens can only be recovered when the updating stage is ended, and for the Crowdsale only after the Crowdsale period has ended and those tokens are not part of the current distribution)

We have a Fundkeeper role which actually follows the OpenZeppelin Ownable pattern. The Fundkeeper holds all of the tokens/ETH of a contract (unless the contract holds these funds itself to distribute them) and so we wanted it to only be a single account not multiple like the other roles.

Fundkeeper - an adapted role like https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/ownership/Ownable.sol

Most contracts have a Manager Role which generally acts like Owner in other contract schemes (generally permissioned to execute admin functions)

We have created interfaces for all major contracts to provide streamlined ABIs for external functionality/interaction

Presale, Bounty, Airdrop - are all the exact same logic, just renaming for allocation clarity

The purpose for these three is to create blockchain records of tokens allocated before the sale in our centralised architecture… only the Manager can update/adjust these records. These contracts accept an allocation of tokens from the ERC-20 Fundkeeper via the allow/transferFrom pattern

After record creation, accounts that have records created can claim their tokens at a rate of 1% per day (or 100 tokens per day) whichever is larger, until all tokens claimed (last claim may claim less than 100, just claims the remaining small balance)

Crowdsale -

Crowdsale has a Whitelister Role in addition to others.

This Crowdsale follows the EOS model with 3 major differences: 1.Whitelisting of accounts for participation - this is to enforce KYC restrictions on the contract level 2.A hidden hard cap - have implemented a hidden hard cap to curb over subscription and to incentivise earlier participation 3.Interval reserve amounts 1.If the interval reserve amount of ETH is not met, only a portion of the interval tokens are distributed and the reserve price goes down by up to three steps depending on how much the interval was off of target, until a floor price (these prices are in ETH per one token) E.g. if the reserve is 10 ETH, and only 5 ETH is contributed, then only 5/10 (or 50% of the interval's tokens are distributed) 2.If the daily reserve is met, all 100% of the interval tokens are distributed and the reserve price goes up by up to three steps, until a ceiling price 3.For this to remain accurate there is ability for the contract Manager Role to adjust the ETH price daily (rebasing of the daily reserves from this price change occurs in the next day) 4.All USD entries are 18 decimal precision (for example USD$1 = 10*18, same as ETH and WEI)

I've found that for testing, these parameters makes the math easier -

ERC-20 mint amount 
100000000000000000000000000 TBN

TBNCrowdsale constructor-
Number of intervals - 100
Guaranteed - 10
hidden cap hash -  0xfa503158889149816a0e0ec7ba7e92c0e67839b6eaf4aeb5da6272d59c3c1d84 , cap “10000000000000000000” secret “1234567” 

TBNCrowdsale initialize -
ETHPrice - 100000000000000000000   = 100 * 10**18 ($100)
reserveFloor -       50000000000000000 = 5 * 10 **16 ($0.05)
reserrveStart - 100000000000000000 = 10 ** 17 ($0.1)
reserveCeiling -      150000000000000000 = 1.5 * 10**17 ($0.15)
Allocation - 1000000000000000000000000 = 10**24 (1mil TBN) 
setRebase - 2000000000000000000000000 ($200)

also changing INTERVAL_BLOCKS = 10 (to reduce the time per interval)

If you have any other questions feel free to ask.

Source code

https://github.com/tubiex/smart-contracts/tree/f162c021c0d3d4c72c2144c80968d5190086300a/contracts

Disclosure policy

terry@tubiex.com

Platform

ETH

Complexity

Medium

MrCrambo commented 5 years ago

Auditing time 4 days

yuriy77k commented 5 years ago

@MrCrambo assigned.

gorbunovperm commented 5 years ago

Estimated auditing time is 7 days.

yuriy77k commented 5 years ago

@gorbunovperm assigned.

gorbunovperm commented 5 years ago

My report is finished.

danbogd commented 5 years ago

Estimated auditing time is 7 days.

yuriy77k commented 5 years ago

@danbogd assigned

danbogd commented 5 years ago

My report is finished.

yuriy77k commented 5 years ago

1. Summary

Tubiex smart contract security audit report performed by Callisto Security Audit Department

ERC-20 implementation, Presale token management (creating records of presales and allowing users to claim those records at a rate of 1% per day), Crowdsale contract - EOS model with 2 additions: 1) a hidden hard cap, and 2) dynamic daily reserve price (with the ability to rebase the ETH price daily to have an accurate reserve.

https://tubiex.com/

2. In scope

Commit hash: f162c021c0d3d4c72c2144c80968d5190086300a.

3. Findings

In total, 2 issues were reported including:

1 low severity issues.
1 minor observation.

No critical security issues were found.

3.1. Known vulnerabilities of ERC-20 token

Severity: low

Description

It is possible to double withdrawal attack. More details here.
Lack of transaction handling mechanism issue. WARNING! This is a very common issue and it already caused millions of dollars losses for lots of token users! More details here.

Recommendation

Add into a function transfer(address _to, ... ) following code:

require( _to != address(this) );

3.2. Deprecated method.

Severity: minor observation

Description

The function () payable { revert(); } was a pattern used to prevent implicit acceptance of ether in Solidity versions older than 0.4.0, but today this is unneeded.