This is the report from a security audit performed on Gigzi smart contract (ETH) by alexo18. The audit focused primarily on the security of funds and fault tolerance of the Gigzi contract. The main intention of this contract is to serve as token ecosystem.
FeeableToken.sol
GigBlack.sol
GigGold.sol
GigSilver.sol
GigPlatinum.sol
Migrations.sol
MessageHelper.sol
In total, 8 issues were reported including:
- 2 high severity issues.
- 2 medium severity issues.
- 4 low severity issues.
There is no parameter which represents the reward period in reward calculations and at the moment time difference in seconds used as number of periods without an additional conversion to desired period.
https://github.com/GigziProject/GigziContracts/blob/master/contracts/GigBlack.sol#L141
Provide a parameter which represents the reward period .
Convert time interval in seconds to desired period as follows :
(now - x ) / (3600 *24) to days for example
Whether using raw calls (of the form someAddress.call()) , assume that malicious code might execute. Even if ExternalContract is not malicious, malicious code can be executed by any contracts it calls.
https://github.com/GigziProject/GigziContracts/blob/master/contracts/helper/MessageHelper.sol#L42
Restrict this function to a list of trusted addresses by using a modifier.
In some specific circumstances,when multiple concurrent calls are made,there is a situation when two or more addresses will have the same value in the 'accountIndexes' mapping.
https://github.com/GigziProject/GigziContracts/blob/master/contracts/GigBlack.sol#L97
Use return value of the 'accounts.push()' instead of direct call to the 'accounts.length' as follows:
accountIndexes[addr] = accounts.push(AccountRewardInfo({.......
....
The 'updateAccountReward' funtion automatically pushes address into the 'rewardAccount[]' in case if address doesn't exist. Such behavior opens the possibility of flooding the 'rewardAccount[]' whith a 'ghost' addresses. While these addresses not pose a direct threat , such attack can affect contract performance .
https://github.com/GigziProject/GigziContracts/blob/master/contracts/GigBlack.sol#L202
execute the 'updateAccountReward' only on addresses with non zero balance as follows:
if (balanceOf(_from)>0) updateAccountReward (_from);
The 'setTxFeeCollector' function not updates the 'accountReserved[]' with new fee collector address, moreover, the function does not removes last collector address from the array.
https://github.com/GigziProject/GigziContracts/blob/master/contracts/FeeableToken.sol#L65
update the 'accountReserved[]' with new collector address ,as follows:
for (uint i=0; i<accountsReserved.length; i++) {
address accountReserved = accountsReserved[i];
if (txFeeCollector == accountReserved) {
accountsReserved[i]=feeCollector;
break;
...........
In the internal calculations the contract converts the 'feeLimit' parameter to decimals , thereby creating premises for uint overflow , like here.
https://github.com/GigziProject/GigziContracts/blob/master/contracts/FeeableToken.sol#L46
The better approach here is to convert the 'feeLimit' parameter to decimals immediately in the 'setFee' function and throw an error in case of overflow.
The contract description implies existence of a positive correlation between the amount of tokens accumulated on collector account and the reward payments in GZG tokens ,which is backed by gold. Absence of such correlator may lead to a situation when not enough gold purchased to back issued GZG tokens.
The 'processTransfer' , 'transferWithFee' , 'transferWithoutFee' functions do not return value explicitly. It is important to explicitly return value from these functions to best atomize the Tx process.
https://github.com/GigziProject/GigziContracts/blob/master/contracts/FeeableToken.sol#L127 https://github.com/GigziProject/GigziContracts/blob/master/contracts/FeeableToken.sol#L142 https://github.com/GigziProject/GigziContracts/blob/master/contracts/FeeableToken.sol#L174
Two critical vulnerabilities were detected. It is highly recommended to fix the bugs before use.