• Smart Contract Security Audit

SMART CONTRACT SECURITY AUDIT: NapoleonX

NapoleonX
Napoleonx is the first crypto asset manager piloting trading bots, it intends to become the master sponsor for launching a series of Decentralized Autonomous Funds (DAFs). These ultimate investment vehicles will invest in 100% algorithmic solutions as proposed by Napoleon Crypto. NapoleonX is most likely to benefit from 85% of all performance fees generated by the various DAFs and 100% of the performance upon its participation in these vehicles. Napoleon Crypto will make available its existing library and will invite access to any future development.

Introduction
Techracer’s Smart Contract Security Auditing team worked on performing the contract audits for NapoleonX platform. We started from scratch in an effort to have a higher-level foundation protocol and a decentralized blockchain platform that incorporates the forefront research in blockchain technology. We developed a mission statement and kick-started with the project to build a foundation by adhering to the components needed. We deployed a team with a expertise in custom-made solutions and delivered an end-to-end security assessment.

The NapoleonX system must stand up to the rigors of attacks from the moment of its ICO. The technical stipulations for the project and security audit for solidity smart contracts are rigid, this involves Design review, Solution review, Code review & Fuzz testing w.r.t:

  • Testing the system
  • Testing the mathematics underlying the system
  • Testing the protocol
  • Testing the architecture
  • Performing a security assessment across all the third-party libraries

Manual Audit:

Style Guide Violations: (these recommendations are not very important, but it will be good if the client resolves them)

  1.    Top level declarations in solidity should have two blank lines before them. An Image has been added from the contract Whitelisting, but similar violation is present in every contract. (ref)
    Top level declarations in solidity
  2. Only spaces should be used, mixed tab + spaces combination is used within contracts. (ref)
  3. Required space after for function. (ref)
    function
  4. There should be a space between if and ().
    constructor function
  5.    Function order incorrect, constructor function should not go after public function. (ref)
    constructor function
  6.    Function order incorrect, public function cannot go after internal function, similar is the case for hasEnded function of crowdsale contract. (ref)
    constructor function
  7.    Two spaces have been used for indentation in Crowdsale Contract, instead four spaces are recommended per indentation level. (ref)
  8.    Following line from Crowdsale Contract breaks three guidelines, suggested by solidity:
  • Line length should be restricted to 120 characters, here it is 128 characters.
  • There should be a single space before a function. (ref)
  • Function order is incorrect, like the last issue.
     Crowdsale Contract breaks
  1. Missing space at the end of contract files.
  2. Visibility modifiers should be first among the list of modifiers in a function declaration. Therefore, in function fund of Pending Contributions public should appear before payable.
    modifiers

Security and gas considerations:

  • Pragma should be locked before deployment in every contract. (ref)

Whitelist –

  1.    Use delete whitelist[candidate] instead of whitelist[candidate] = false to save gas
  2.    In function authorize Many () instead of passing an array of fixed size of 50 we should pass dynamic sized array. It will add more flexibility to the code. Also, there will not be much difference in the amount of gas consumed. But keep in mind to have an array size compatible with the block gas limit.
  3.    No need for the following function, solidity automatically creates a public value returning function for public variables, a better way would be to rename the mapping whitelist to is Whitelisted and remove this function:
    solidity automatically

Pending Contributions-

  1.    Integer Overflow can happen in fund function of this contract, Safemath library is inherited but it’s add function is not used. If we use:

contributions [contributor]=contributions[contributor].add(msg.value);
instead of :

contributions[contributor] += msg.value;     Integer overflow can be avoided.

WhitelistedGateway-

  1. Integer Overflow can happen in fund function of this contract,

Safemath library is inherited but it’s add function is not used. If we use :

contributions[contributor]= contributions[contributor].add(msg.value);
instead of :

contributions[contributor] += msg.value;    Integer overflow can be avoided.

  1. No event emitted after adding owner, in the following function:

Instead an event like the following should be emitted:

event OwnerAdded(address indexed newOwner);

Crowdsale-

  1. SafeMath library has been inherited but not used in the contract.

Automated Audit:

Securify:

Audit report from automated analysis of Securify.
Automated Audit

 

Mythril

Mythril approves the contracts and displays no error/warning.

Oyente

Oyente’s report for Whitelist Contract:

Oyente’s report for WhitelistedGateway Contract:
Automated Audit

Oyente’s report for PendingContributions Contract:
Automated Audit

Oyente’s report for Crowdsale Contract:

Recommendations:
Pending Contributions-

  • Instead of having a whole different pending contributions contract, simply have all the logic inside the whitelistGateway contract.
    • This separation does not offer security functionality, nor is the contract code too heavy to be deployed in a single deployment
    • We are deploying the contracts whitelist Gateway and pending Contributions contracts at the same time so there is no need to separate them. Combining these two contracts will reduce gas consumption
    • It only increases gas costs due to the communication between different contracts
    • A lot of gas can be saved also in crowdsale contract

Crowdsale-
Unnecessary imports, were found in the document: This was used in

Crowdsale contract (similar unnecessary imports in other contracts):Automated Audit

But this would suffice:

Automated Audit

No indexed events
There is no use of indexed events in any of the given contracts. It will make it difficult to administer the events. The indexed parameters for logged events allows us to search for these events using the indexed parameters as filters.

Conclusion
The resulting report on several issues from grammar to few medium importance vulnerabilities. Each issue stated was discussed with the client and the contracts were sent for the update.

After the revision, the new version of contracts was assessed in which no vulnerabilities were found.

2018-02-14T10:56:55+00:00 February 14th, 2018|Blockchain Tutorials, Blog|