Ethereum GigziContracts audit report

ETH_GigziContracts_audit_report.md

Ethereum GigziContracts audit report

Summary

This is the report from a security audit performed on GigziContracts by gorbunovperm.

The smart contract allows the Central Authority (CA) to issue tokens backed by precious metals. Tokens have a changeable tx fee. This tx fee is accumulated on CA account. All users who hold GZB tokens on their accounts will receive payments in GZG from the CA; a size of the payment will be proportional to the amount of GZB tokens on the account.

In scope

1. FeeableToken.sol
2. GigBlack.sol
3. GigCrowdsale.sol
4. GigGold.sol
5. GigPlatinum.sol
6. GigSilver.sol
7. Migrations.sol
8. MessageHelper.sol

Excluded

1. openzeppelin-solidity/contracts/math/SafeMath.sol
2. openzeppelin-solidity/contracts/ownership/Ownable.sol
3. openzeppelin-solidity/contracts/token/ERC827/ERC827Token.sol
4. openzeppelin-solidity/contracts/token/ERC20/BurnableToken.sol
5. openzeppelin-solidity/contracts/token/ERC20/MintableToken.sol
6. openzeppelin-solidity/contracts/crowdsale/validation/CappedCrowdsale.sol
7. openzeppelin-solidity/contracts/crowdsale/validation/TimedCrowdsale.sol

Findings

In total, 4 issues were reported including:

• 0 high severity issue.

• 0 medium severity issues.

• 2 low severity issues.

• 2 minor observations.

Security issues

Issues for FeeableToken.sol

1. It is desirable to use view modifier

Severity: minor observation

Code snippet

function isFeeShouldBePaid(address _from, address _to) internal returns (bool) {
    // ...       
    return true;
}

Description

This function is just reading data but not modify. For code understanding reason it should be marked as view.

Recomendation

Just add view modifier.

2. After change txFeeCollector address, the old address will still not pay the commission.

Severity: low

Code snippet

function setTxFeeCollector(address feeCollector) public onlyOwner returns (bool success) {
    txFeeCollector = feeCollector;
    return true;
}

Description

After changing the txFeeCollector address, it should be freed from fees on transactions. And the old txFeeCollector should be removed from reserved accounts.

Recomendation

Change the implementation of setTxFeeCollector method. Remove the old txFeeCollector address from accountsReserved array and add there new txFeeCollector address.

Issues for GigBlack.sol

3. The reward will be lost after burning tokens.

Severity: low

Code snippet

function burn(uint256 _value) public {

    require(_value > 0);
    require(_value <= balances[msg.sender]);

    address burner       = msg.sender;
    balances[burner]     = balances[burner].sub(_value);
    totalSupply_         = totalSupply_.sub(_value);

    updateAccountReward (burner);

    Burn                (burner, _value);
}

Description

It would be more logical to update a reward of account before burning tokens on this address to preserve the deserved reward.

Recomendation

Move the call to the updateAccountReward method before subtraction of the balance at the address.

4. Unnecessary operations can be performed.

Severity: minor observation

Code snippet

function getAccountReward(address addr) public view returns (uint, uint, uint)
{
    if (!hasAccount(addr)) {
        return (0,0,now);
    }

    // Some operations that can be performed in vain.
    // ...

    if (supplyTimeTotal == 0)
        return (0,0,now);

    return (rewardAccum, supplyTimeTotal, now);
}

Description

In case if (supplyTimeTotal == 0) condition is true it will be return the empty result and the code above executed in vain.

Recomendation

Condition if (supplyTimeTotal == 0) should be moved to the top of the method.

Conclusion

This contracts has several minor issues. And in general, its security is at a good level.