Cryptosolartech Security Audit

Coinfabrik’s smart contract audit’s team was asked to audit the contracts for the Cryptosolartech sale. Firstly, we will provide a summary of our discoveries and secondly, we will show the details of our findings.

Summary

The contracts audited are from the Cryptosolartech repository. The audit is based on the commit 4a04863c1cf6ef55d89a3ed63c3e0de863efe4da and updated to reflect the changes made on commit bd4277db249d9550cea439f9044c54882c379d5d.

The audited contracts are:

• contracts/Burnable.sol: Burnable token interface.
• contracts/Crowdsale.sol: Deployable contract for the crowdsale.
• contracts/CrowdsaleToken.sol: Deployable contract for the token.
• contracts/DeploymentInfo.sol: View functions for additional information.
• contracts/EIP20Token.sol: EIP20 Token interface.
• contracts/GenericCrowdsale.sol: Abstract base contract for token sales.
• contracts/Haltable.sol: Inheritable contract with modifiers for halting capabilities.
• contracts/LostAndFoundToken.sol: Inheritable contract for securely retrieving mistakenly sent tokens.
• contracts/Mintable.sol: Mintable token interface.
• contracts/MintableToken.sol: Inheritable contract for minting capabilities.
• contracts/Ownable.sol: Inheritable contract with modifiers for privilege calls.
• contracts/ReleasableToken.sol: Inheritable contract
• contracts/SafeMath.sol: Overflow checked arithmetic functions
• contracts/StandardToken.sol: Standard ERC20 token implementation.
• contracts/TokenTranchePricing.sol: Tranche pricing capabilities for the crowdsale.
• contracts/UpgradeableToken.sol: Inheritable contract for upgrading capabilities.
• contracts/UpgradeAgent.sol: Inheritable contract for the agent executing the upgrade.

The following analyses were performed:

• Misuse of the different call methods: call.value(), send() and transfer().
• Integer rounding errors, overflow, underflow and related usage of SafeMath functions.
• Old compiler version pragmas.
• Race conditions such as reentrancy attacks or front-running.
• Misuse of block timestamps, assuming anything other than them being strictly increasing.
• Contract softlocking attacks (DoS).
• Potential gas cost of functions being over the gas limit.
• Missing function qualifiers and their misuse.
• Fallback functions with a higher gas cost than the one a transfer or send call allows.
• Fraudulent or erroneous code.
• Code and contract interaction complexity.
• Wrong or missing error handling.
• Overuse of transfers in a single transaction instead of using withdrawal patterns.
• Insufficient analysis of function input requirements.

Detailed findings

Minor severity

Unintended multiplication by 0 in calculateTokenAmount()

It is possible for a user to buy tokens but receive nothing, as milieurs_per_eth isn’t updated in the constructor, which means that transfer will call calculateTokenAmount() and multiply it by 0 (being the value of tokensPerEth). The user will have sent money to the contract but received no tokens.

function calculateTokenAmount(uint weiAmount, address) internal view returns (uint     weiAllowed, uint tokenAmount)
 {
   uint tokensPerEth = getCurrentPrice(tokensSold).mul(milieurs_per_eth).div(1000);
   //...
 }

This could easily be fixed by adding the following to the function:

function calculateTokenAmount(uint weiAmount, address) internal view returns (uint     weiAllowed, uint tokenAmount)
 {
   require(milieurs_per_eth != 0);
   uint tokensPerEth = getCurrentPrice(tokensSold).mul(milieurs_per_eth).div(1000);
   //...
 }

This situation can happen only if function updateEursPerEth wasn’t called because function updateEursPerEth checks if the price is zero.

Finalize does not call burn() function

At Crowdsale.sol, in the finalize function, unsold tokens aren’t burned:

function finalize() public inState(State.Success) onlyOwner stopInEmergency
{
  //Tokens sold + bounties represent 75% of the total, the other 25% goes ti the multisig to the partners and to regulate market
  uint sold = tokensSold.add(  initial_tokens);
  uint toShare = sold.mul(25).div(75).mul(10**uint(token.decimals()));
  token.setMintAgent(address(this), true);
  token.mint(multisigWallet, toShare);
  token.setMintAgent(address(this), false);
  token.releaseTokenTransfer();
  super.finalize();
}

This means that the CrowdsaleToken contract stores the address of the Crowdsale as if it had tokens. Since they are not going to be used, it is best to simply dispose of them.

This issue in the commit 4a04863c1cf6ef55d89a3ed63c3e0de863efe4da.

Enhancements

Use of updated pragmas

In later commits, the contracts use the following pragma:

pragma solidity ^0.4.19;

We recommend updating compiler directives so that newer compiler versions are used, as the current solidity version is 0.4.21. In the same vein, events are now emitted using the emit keyword, which results in more code legibility. This also helps to differentiate between function calls and event emission.

Conclusion 

The contracts followed recommended practices and were acceptably documented. Common attacks like the EIP 20 approve/transferFrom attack were taken into account. Two errors were found for either undefined or unaccounted behavior. One was properly fixed. The other was addressed calling function updateEursPerEth during the Smart Contract deployment so the price can never be zero.