KEYis Smart Contracts Audit Report

QuillHash Team
QuillHash
Published in
7 min readJan 24, 2019

We’ve been asked by the KEYis team to review and audit their smart contracts.

We at QuillAudits are a team of blockchain developers , consultants and security auditors , trying to build a more secure and safe community in the Blockchain space. QuillAudits, QuillHash Smart Contracts Security Audit platform ensures the reliability of your smart contract by complete assessment of your system’s architecture and your smart contract codebase.

The Code is located in the KEYis github Repository and the version used for this commit is. d30a969dbc9df61b295e33a8201235d64611a2a1

by QuillAudits, 05 January 2019

Introduction :

This Audit Report highlights the overall security of KEYis Smart Contract. With this report, we have tried to ensure the reliability of their smart contract by complete assessment of their system’s architecture and the smart contract codebase.

Auditing Approach and Methodologies applied :

Quillhash team has performed thorough testing of the project starting with analyzing the code design patterns in which we reviewed the smart contract architecture to ensure it is structured and safe use of third party smart contracts and libraries.

Our team then performed a formal line by line inspection of the Smart Contract in order to find any potential issue like race conditions, transaction-ordering dependence, timestamp dependence, and denial of service attacks.

In the Unit testing Phase we coded/conducted Custom unit tests written for each function in the contract to verify that each function works as expected. In Automated Testing, We tested the Smart Contract with our in-house developed tools to identify vulnerabilities and security flaws.

The code was tested in collaboration of our multiple team members and this included -

  1. Testing the functionality of the Smart Contract to determine proper logic has been followed throughout.
  2. Analyzing the complexity of the code by thorough, manual review of the code, line-by-line.
  3. Deploying the code on testnet using multiple clients to run live tests
  4. Analyzing failure preparations to check how the Smart Contract performs in case of bugs and vulnerabilities.
  5. Checking whether all the libraries used in the code are on the latest version.
  6. Analyzing the security of the on-chain data.

Security Level references

Every issue in this report was assigned a severity level from the following:

High severity issues will probably bring problems and should be fixed

Medium severity issues could potentially bring problems and should eventually be fixed.

Low severity issues are minor details and warnings that can remain unfixed but would be better fixed at some point in the future.

High severity issues:-

  1. Token amount is not multiplied by 1 ether or 1⁰¹⁸ to get sufficient amount of tokens, that’s why after calling buyToken function by whitelisted user having value 1 ether, investor is not be able to get 2000 tokens, they are only getting0.00000000000002000

Status : fixed by developer

2. Test case fail of approving negative tokens, it should not approve negative tokens.

Status : fixed by developer

Medium Severity Issues:-

  1. Avoid state change after transfer function or external calls token sale contract line no 225 must be before transfer function, Reentrancy Issue.

Status : fixed by developer

LowSeverity Issues:-

1.) Solidity version must be fixed(Always use latest Version).

It should not pragma solidity ^0.4.23;It should be pragma solidity 0.4.23;

Status : fixed by developer

2.) you can remove event emit at line no 229 as already transfer function contain event emit, at etherscan event is listen and it’s showing ERC20 token is transferred to two address, it means it’s listening two transfer event even though both are same still you can remove from line no 229.

Status : fixed by developer

3.) your specification document contain compiler version 0.4.21, and you are using ^0.4.23 make sure you are using same resources, compilation version and libraries that are listed in your paper.

Status : fixed by developer

Review Audit Contains Following Issues :

High severity issues:-

1) Logic for buyToken() is not correct if i send ether sufficient to buy all the tokens in one go, then token sold in tier 1 is 15000000, token sold in tier 2 is 135000000 and token sold in tier 3 is zero .

Also if you will not end Sale immediately when all the sale token is sold then by participating in a sale, investor will drain all the tokens in contract and you will not be able to end Sale as team and other will not be able to get sufficient tokens

Status : Not fixed yet

Solution :You are updating all variables locally like tier = 0 always, you can globally update variable and every time buy function is called tier will be start from current tier in automatic tier change.

{ tokensSold[tier + 1] = tokensSold[tier + 1].add(leftoverQuantity); }

Will add tokens to next tier but the line shown below is out of if() scope and will check token sold in previous tier not the current tier.

require(tierToLimits[tier].sub(tokensSold[tier]) >= quantity);

You should check token sold in current tier using require condition immediately after balances is updated in that tier and in same if() scope.

Also avoid transfer of tokens in fractions at line 199, you are transferring tokens that are remaining in that tier and then before checking next tier left tokens, balance is update you can hold the remaining tokens in a local variable, and check further before transferring tokens, if next tier have sufficient amount of tokens to transfer or not.

balances[msg.sender] = balances[msg.sender].add(leftoverQuantity);balances[address(this)] = balances[address(this)].sub(leftoverQuantity);

Check tokens left in that tier before updating balance.

require(tierToLimits[tier].sub(tokensSold[tier])>=quantity or leftoverquantity);tokensSold[tier].add(quantity or leftoverquantity);

Use require after updating balance

require(tierToLimits[tier] >= (tokensSold[tier]);

2.) As you are not following above require conditions in correct manner and not updating variables in their scope, this raised a big issue, if you will fail to end sale using endSale() function immediately after tokens for investors is sold completely, token that allotted for team and cost will be sold to investors as tokens are in contract and tier 3 will not reach and tokens will be directly drawn by investors, and neither you will be able to end sale nor tokens for team and cost will be send to them.

Status : Not fixed yet

Use solution given in high severity issue 1 to solve this issue also you can use below given require statement.

require( getTokensSold() <= investorAlloc );

Unit Testing

Test Suite Result

✓ Should correctly initialize constructor values of Contract (1612ms)

✓ Should change Owner account from accounts[0] to accounts[9] (456ms)

✓ Should change Owner account from accounts[9] to accounts[0] (226ms)

✓ Should change cost allocation token account from accounts[0] to accounts[1] (363ms)

✓ Should change Team allocation token account from accounts[0] to accounts[2] (231ms)

✓ Should change withdraw wallet account from accounts[0] to accounts[3] (302ms)

✓ Should whitelist accounts[4] (286ms)

✓ Should whitelist accounts[6] (385ms)

✓ Should whitelist accounts[5] (290ms)

✓ Should whitelist accounts[8] (320ms)

✓ Should buy correct Tokens when manual tier is off accounts[4] (870ms)

✓ Should buy correct Tokens when manual tier is off accounts[5] (604ms)

✓ Should buy correct Tokens when manual tier is off accounts[6] (1094ms)

✓ pause and get sale status (191ms)

✓ unpause and get sale status (200ms)

✓ Should Burn tokens (330ms)

✓ Should not Burn Negative tokens (187ms)

✓ Should remove whitelisted accounts[1] (295ms)

✓ should Approve address to spend specific token (178ms)

✓ should not Approve address to spend negative tokens (125ms)

✓ should not be able to transfer negative approved tokens (71ms)

Implementation Recommendations

Now Contract doesn’t contain issues that are listed in initial report, KEYis development team shows great enthusiasm to solve issues and make contract more secure but some of the functionalities were not tested in initial audit because of some high severity issues that needed to be fixed before testing further. As now KEYis development team have fixed all issues and we were able to test further, Our Audit team found some high severity issues that need to be solve before deploying on main net, all the issues with their possible solution is listed with recommendations.

Comments:-

Overall, the code is clearly written, and demonstrates effective use of abstraction, separation of concerns, and modularity. KEYis development team demonstrated high technical capabilities, both in the design of the architecture and in the implementation.

We found some critical issue and several additional issues that require the attention of the KEYis team. Given the subjective nature of some assessments, it will be up to the KEYis team to decide whether any changes should be made.

Previous hash commit used to Audit

e323342db359e18f532392b0d4d4d733ff4b0566

Transaction Hash of all the test performed on test network (Rinkeby).

https://docs.google.com/spreadsheets/d/1NJY_mLfpPCzsbpD6l1DfgaB4whxcXofxdk4dTfN_c5Q/edit?usp=sharing

Comments:

Overall, the code is clearly written, and demonstrates effective use of abstraction, separation of concerns, and modularity. KEYis development team demonstrated high technical capabilities, both in the design of the architecture and in the implementation.

We found some critical issue and several additional issues that require the attention of the KEYis team. Given the subjective nature of some assessments, it will be up to the KEYis team to decide whether any changes should be made.

You can request for Audit by filling a form :

https://quillhash.typeform.com/to/KQ5Hhm

Thanks for reading. Also do check out our earlier blog posts.

QuillAudits is a secure smart contract audits platform designed by QuillHash Technologies. It is a fully automated platform to verify smart contracts to check for security vulnerabilities through it’s superior manual review and automated tools. We conduct both smart contract audits and penetration tests to find potential security vulnerabilities which might harm the platform’s integrity.

To be up to date with our work, Join Our Community :-

Telegram| Twitter| Facebook| LinkedIn

--

--