2key network

[carlossampol]

Audit request

Smart contracts allowing Social Sourcing - Incentivised Activation of Result-Driven online virality. More can be found on 2key.network (litepaper, whitepaper etc.)

Source code

https://github.com/2key/contracts/tree/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key

Disclosure policy

security@2key.network

Platform

Ethereum

Number of lines:

7728

Dates

Audit started 7/22/2019

Audit deadline 8/13/2019

[Dexaran]

Special auditing conditions

I think that we must apply special conditions and define a special set of rules for audit of large scale contracts (>1 week auditing time).

I propose the following plan for the large scale smart-contract audit:

1. I'm placing a total reward pool of 1,500,000 CLO for this contract.

2. All four auditors will review the code within the next 23 days. Once all the reports are completed or the specified time expire we will calculate the results based on the results of severity points found be each auditor.

3. Each auditor will receive payment according to his share of reported findings.

2key project is running a bugbounty. We ask auditors to describe any found bugs in their reports but not submit this bugs for the bugbounty. Instead, we will submit the bugs for the bugbounty once all the reports are finished. If a bug will be confirmed and 2key team will pay us the reward then we will deliver 100% of the reward amount to the first auditor who described the bug in his report. Then we will pay an additional 10% of the bugbounty reward amount (in CLO) for staying compatible wiht our rules to auditors who described the confirmed bug.

[gorbunovperm]

@Dexaran Please specify a deadline in the request.

[gorbunovperm]

The audit started. A link to the report template has been sent to the Manager.

[MrCrambo]

Audit started. Link to the report has been sent to Manager.

[yuriy77k]

@gorbunovperm @MrCrambo assigned

[danbogd]

The audit started. A link to the report template has been sent to the Manager.

[yuriy77k]

@danbogd assigned

[RideSolo]

@yuriy77k gist generated, waiting for assignment Also I see that 2Key team updated the smart contracts recently ? how do we deal with this situation since previous auditor could find an issue that doesn't exist anymore.

[yuriy77k]

@RideSolo assigned

[yuriy77k]

@yuriy77k gist generated, waiting for assignment Also I see that 2Key team updated the smart contracts recently ? how do we deal with this situation since previous auditor could find an issue that doesn't exist anymore.

let's use this commit https://github.com/2key/contracts/tree/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key

[yuriy77k]

@danbogd @gorbunovperm @RideSolo please, pay attention, you have to complete the audit in 3 days (15 August)

[gorbunovperm]

My report is finished.

[MrCrambo]

My report is finished

[RideSolo]

@yuriy77k I started the audit late compared to other auditors, I won't finish the 15th.

[danbogd]

My report is finished.

[yuriy77k]

2key network Security Audit Report

1. Summary

2key network smart contract security audit report performed by Callisto Security Audit Department

2. In scope

Commit hash c85f3e1f3a04c56afd28bf673758367cc3df6609.

• TwoKeyAcquisitionCampaignERC20.sol.

• TwoKeyAcquisitionLogicHandler.sol.

• TwoKeyConversionHandler.sol.

• TwoKeyPurchasesHandler.sol.

• ArcERC20.sol.

• TwoKeyCampaign.sol.

• TwoKeyCampaignIncentiveModels.sol.

• ERC20.sol.

• InvoiceTokenERC20.sol.

• TwoKeyDonationCampaign.sol.

• TwoKeyDonationCampaignType.sol.

• TwoKeyDonationConversionHandler.sol.

• TwoKeyDonationLogicHandler.sol.

• ITwoKeyAdminStorage.sol.

• ITwoKeyBaseReputationRegistryStorage.sol.

• ITwoKeyCampaignValidatorStorage.sol.

• ITwoKeyCommunityTokenPoolStorage.sol.

• ITwoKeyDeepFreezeTokenPoolStorage.sol.

• ITwoKeyEventSourceStorage.sol.

• ITwoKeyExchangeRateContractStorage.sol.

• ITwoKeyFactoryStorage.sol.

• ITwoKeyLongTermTokenPoolStorage.sol).

• ITwoKeyMaintainersRegistryStorage.sol.

• ITwoKeyPlasmaEventsStorage.sol.

• ITwoKeyPlasmaMaintainersRegistryStorage.sol.

• ITwoKeyPlasmaRegistryStorage.sol.

• ITwoKeyRegistryStorage.sol.

• ITwoKeySignatureValidatorStorage.sol.

• ITwoKeyUpgradableExchangeStorage.sol.

• IDecentralizedNation.sol.

• IGetImplementation.sol.

• IERC20.sol.

• IHandleCampaignDeployment.sol.

• IKyberNetworkProxy.sol.

• IMaintainingPattern.sol.

• IStructuredStorage.sol.

• ITwoKeyAcquisitionARC.sol.

• ITwoKeyAcquisitionCampaignERC20.sol.

• ITwoKeyAcquisitionCampaignStateVariables.sol.

• ITwoKeyAcquisitionLogicHandler.sol.

• ITwoKeyAdmin.sol.

• ITwoKeyBaseReputationRegistry.sol.

• ITwoKeyCampaignGetReferrers.sol.

• ITwoKeyCampaignPublicAddresses.sol.

• ITwoKeyCampaignValidator.sol.

• ITwoKeyConversionHandler.sol.

• ITwoKeyConversionHandlerGetConverterState.sol.

• ITwoKeyDonationCampaign.sol.

• ITwoKeyDonationCampaignFetchAddresses.sol.

• ITwoKeyDonationConversionHandler.sol.

• ITwoKeyDonationLogicHandler.sol.

• ITwoKeyEventSource.sol.

• ITwoKeyEventSourceEvents.sol.

• ITwoKeyExchangeRateContract.sol.

• ITwoKeyMaintainersRegistry.sol.

• ITwoKeyPlasmaEvents.sol.

• ITwoKeyPlasmaRegistry.sol.

• ITwoKeyPurchasesHandler.sol.

• ITwoKeyReg.sol.

• ITwoKeyRegistry.sol.

• ITwoKeyRegistryEvents.sol.

• ITwoKeySingletoneAddressStorage.sol.

• ITwoKeySingletoneRegistryFetchAddress.sol.

• ITwoKeySingletonesRegistry.sol.

• ITwoKeyWeightedVoteContract.sol.

• IUpgradableExchange.sol.

• Call.sol.

• GetCode.sol.

• IncentiveModels.sol.

• SafeERC20.sol.

• SafeMath.sol.

• Utils.sol.

• ITwoKeySingletonUtils.sol.

• StandardTokenModified.sol.

• TwoKeyAdmin.sol.

• TwoKeyBaseReputationRegistry.sol.

• TwoKeyCampaignValidator.sol.

• TwoKeyCongress.sol.

• TwoKeyEconomy.sol.

• TwoKeyEventSource.sol.

• TwoKeyExchangeRateContract.sol.

• TwoKeyFactory.sol.

• TwoKeyLockupContract.sol.

• TwoKeyMaintainersRegistry.sol.

• TwoKeyPlasmaEvents.sol.

• TwoKeyPlasmaMaintainersRegistry.sol.

• TwoKeyPlasmaRegistry.sol.

• TwoKeyPlasmaSingletoneRegistry.sol.

• TwoKeyRegistry.sol.

• TwoKeySignatureValidator.sol.

• TwoKeySingletonesRegistry.sol.

• TwoKeyUpgradableExchange.sol.

• TokenPool.sol.

• TwoKeyCommunityTokenPool.sol.

• TwoKeyDeepFreezeTokenPool.sol.

• TwoKeyLongTermTokenPool.sol.

• Proxy.sol.

• StructuredStorage.sol.

• UpgradabilityStorage.sol.

• UpgradeabilityProxy.sol.

• Upgradeable.sol.

• ProxyCampaign.sol.

• UpgradabilityCampaignStorage.sol.

• UpgradeableCampaign.sol.

• DecentralizedNation.sol.

• ERC20CustomToken.sol.

• ERC20TokenMock.sol.

• Ownable.sol.

• TwoKeyAirdropCampaign.sol.

• TwoKeyConversionStates.sol.

• TwoKeyTypes.sol.

• TwoKeyVoteToken.sol.

• UpgradabilityProxyAcquisition.sol.

3. Findings

In total, 27 issues were reported including:

• 5 high severity issues.

• 9 medium severity issues.

• 6 low severity issues.

• 1 notes.

• 6 owner privileges (the ability of an owner to manipulate contract, may be risky for investors).

3.1. No checking for zero address

Severity: low

Description

Incoming addresses should be checked for an empty value(0x0 address) to avoid loss of funds or blocking some functionality.

Code snippet

• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/TwoKeyAirdropCampaign.sol#L88
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/token-pools/TokenPool.sol#L24-L25
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/token-pools/TwoKeyCommunityTokenPool.sol#L21-L23
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/token-pools/TwoKeyCommunityTokenPool.sol#L52
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/donation-campaign-contracts/TwoKeyDonationCampaign.sol#L38
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/donation-campaign-contracts/TwoKeyDonationConversionHandler.sol#L82
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/acquisition-campaign-contracts/TwoKeyAcquisitionCampaignERC20.sol#L45
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyEventSource.sol#L134
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeySingletonesRegistry.sol#L55
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/upgradability/StructuredStorage.sol#L39
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/upgradability/UpgradabilityProxy.sol#L26

3.2. ERC20 Compliance.

Severity: low

Description

According to ERC20 standard, when initializing a token contract if any token value is set to any given address a transfer event should be emitted.

Code snippet

• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/ERC20CustomToken.sol#L12
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/ERC20TokenMock.sol#L12
• https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/acquisition-campaign-contracts/TwoKeyAcquisitionCampaignERC20.sol#L78
• https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyEconomy.sol#L40#L43
• https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/donation-campaign-contracts/InvoiceTokenERC20.sol#L36

3.3. Owner Privileges

Severity: owner privileges

Description

Contract owner allow himself to:

• upgrade contract and implement any logic in the new contract. And even if the new contract will be audited, at any time possible to change the address of the new contract again to not audited and insecure.

• to freeze all transfers on ERC for some period of time

• Owner can change any uint value in TwoKeyUpgradableExchange contract as buy and sell rates, weiRaised values, etc.

• Owner can change proxy logic contract any times and chan change it to not audited contract.

3.4. Wrong value passing

Severity: medium

Description

In function setInitialParams there is wrong value passing to function setInitialParameters(_erc20Address, TWO_KEY_SINGLETON_REGISTRY);, because in all other similar functions there is passing value from arguments of functions and also TWO_KEY_SINGLETON_REGISTRY variable not initialized in this contract.

Recommendation

There should be passed twoKeySingletonesRegistry instead of TWO_KEY_SINGLETON_REGISTRY.

setInitialParameters(_erc20Address, twoKeySingletonesRegistry);

## 3.5. Multi Transfer arrays length check.

### Severity: low

### Description

There are several input arrays, but no check for the same length. In this case it is possible to skip some parameters.

### Code snippet

- https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyCongress.sol#L108-L109
- https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyExchangeRateContract.sol#L61
- https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/DecentralizedNation.sol#L117

## 3.6. Wrong membership address replacing

### Severity: medium

### Description

In function [`replaceMemberAddress`](https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyCongress.sol#L180) there is setting old member info to new address in [line 199](https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyCongress.sol#L199), but `memberAddress` will still be old address, which should be replaced.

### Code snippet

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyCongress.sol#L199

### Recommendation

Create new `Member` structure for replaced address with copying `memberSince`, `votingPower` and `name`.

## 3.7. Wrong `transferFrom` function

### Severity: note

### Description

In function [`transferFrom`](https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/campaign-mutual-contracts/TwoKeyCampaign.sol#L59) there is adding `conversionQuota` to `_to` address balance, where `conversionQuota` is `maximal ARC tokens that can be passed in transferFrom`, but `value` will always be equal to 1. So in this function there will not be transferring tokens from one address to another, there will be burning from one and minting to another.

### Code snippet

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/campaign-mutual-contracts/TwoKeyCampaign.sol#L59

## 3.8. No checking for enough tokens

### Severity: note

### Description

There is no checking that there will be enough tokens in [line 344](https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/acquisition-campaign-contracts/TwoKeyAcquisitionCampaignERC20.sol#L344) in function [`buyTokensAndDistributeReferrerRewards`](https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/acquisition-campaign-contracts/TwoKeyAcquisitionCampaignERC20.sol#L325) as wrote in `TODO` comment.
```solidity
    //TODO: add require that there's enough tokens at this moment

Code snippet

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/acquisition-campaign-contracts/TwoKeyAcquisitionCampaignERC20.sol#L344

Recommendation

Add checking that there is enough tokens.

3.9. Wrong condition checking

Severity: medium

Description

In the function isCampaignEnded there is possibility that campaignRaisedAlready will be more than campaignHardCapWei so in this case this function will return false instead of true.

Code snippet

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/acquisition-campaign-contracts/TwoKeyAcquisitionLogicHandler.sol#L280

Recommendation

Check that campaignRaisedAlready is at least equal to campaignHardCapWei.

if(endCampaignWhenHardCapReached == true && campaignRaisedAlready >= campaignHardCapWei) {
    return true;
}

3.10. Empty if statement

Severity: note

Description

If statement in constructor has no code inside of it and could be deleted.

Code snippet

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/TwoKeyAirdropCampaign.sol#L94

3.11. Anyone can transfer others tokens

Severity: high

Description

Using function transferFrom anyone can transfer other addresses voting points to himself. There is no checking for person who call this function, so he can pass his address as to address and transfer himself anyone's voting points.

Code snippet

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/TwoKeyVoteToken.sol#L109

Recommendation

Better to use transfer function so only voting points' owners will be able to transfer their voting points.

3.12. Allowance function show wrong value

Severity: medium

Description

Function allowance show balanceOf functions' result and doesn't show the amount of tokens that an owner allowed to a spender. as wrote in comment above function.

Code snippet

https://github.com/2key/contracts/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/TwoKeyVoteToken.sol#L135

3.13. Distribution date changing

Severity: medium

Description

In function changeTokenDistributionDate the shift value will be positive if _newDate less then tokenDistributionDate and negative otherwise. Therefore this value should be subtracted from tokenUnlockingDate instead of added.

Same issue occur in function changeDistributionDate in TwoKeyPurchasesHandler.sol contract

Code snippet

        uint shift = tokenDistributionDate - _newDate;
        // If the date is changed shifting all tokens unlocking dates for the difference
        for(uint i=0; i<numberOfVestingPortions+1;i++) {
            tokenUnlockingDate[i] = tokenUnlockingDate[i] + shift;
        }

Recommendation

Use subtraction shift instead of addition:

        uint shift = tokenDistributionDate - _newDate;
        // If the date is changed shifting all tokens unlocking dates for the difference
        for(uint i=0; i<numberOfVestingPortions+1;i++) {
            tokenUnlockingDate[i] = tokenUnlockingDate[i] - shift;
        }

3.14. Not complete removal of a user from Congress

Severity: low

Description

There are no restrictions for adding same user several times. In this case during the user removing only first record will be removed from allMembers array. The rest of the user records will be saved.

Code snippet

• addMember()

3.15. Failure to delete a user is possible

Severity: high

Description

It may happen that the user cannot be removed in case if block gas limit is reached. When removed, two cycles are used and when a large number of users will not have enough gas to perform the function. And considering vulnerability 3.14. an attacker can intentionally spam an array of users.

Code snippet

• removeMember()

3.16. There is no way to remove user

Severity: high

Description

1. Only the contract itself can call the removeMember function. But the contract does not have this call, respectively, there is no way to remove the user.
2. The removeMember function contains this code:

   for (uint j = i; j< allMembers.length; j++){
       allMembers[j] = allMembers[j+1];
   }

   But the last assignment will not be performed. Because if the array consists of 5 elements then allMembers[4] = allMembers[5] expression attempts to access a nonexistent element(last index is 4). And transaction will be reverted.

Note: Also access to nonexistent element is performed here.

Code snippet

1. msg.sender == address(this)
2. a non-existent element

3.17. Anyone can get access to onlyMember functions

Severity: high

Description

Anyone can call replaceMemberAddress function and there is no check of membership. Therefore, an attacker can add any address to isMemberInCongress array and get access to all functions with onlyMember modifier.

Code snippet

• isMemberInCongress = true

3.18. Underflow is possible

Severity: medium

Description

When removing a member, there is no safe mathematical operation maxVotingPower-= votingPower. The following actions may cause an underflow:

1. addMember 0x1, maxVotingPower = 10
2. removeMember 0x1, maxVotingPower = 10
3. replaceMemberAddress 0x1 to 0x2, maxVotingPower = 10 (look at issue 3.17 where the removed user can restore his membership with his votingPower)
4. removeMember 0x2, maxVotingPower = 0-10 (underflow)

And the same scheme underflow with minimumQuorum variable.

Code snippet

• maxVotingPower underflow
• minimumQuorum underflow

3.19. Multiplication after division

Severity: note

Description

Solidity operates only with integers. Thus, if the division is done before the multiplication, the rounding errors can increase dramatically.

Code snippet

• Multiplication after division

3.20. Adding a member is possible only from constructor

Severity: note

Description

The addMember function is used condition:

  if(initialized == true) {
      require(msg.sender == address(this));
  }

But after the initialization there is no way to use addMember because it is not being called from anywhere else in the contract.

Code snippet

• Condition

3.21. Unused contract

Severity: note

Description

TwoKeyVoteToken contract is created but never used:

  votingToken = new TwoKeyVoteToken(address(this));

Code snippet

• new TwoKeyVoteToken

3.22. Known vulnerabilities of ERC-20 token

Severity: low

Description

• It is possible to double withdrawal attack. More details here

• Lack of transaction handling mechanism issue. WARNING! This is a very common issue and it already caused millions of dollars losses for lots of token users! More details here

Code snippet

• InvoiceTokenERC20.sol
• TwoKeyEconomy.sol

Recommendation

Add into a function transfer(address _to, ... ) following code:

require( _to != address(this) );

3.23. Out of Gas

Severity: medium

Description

The contracts have loops the number of iterations of which depends on the variables in the storage and can increase over time. This can lead to "Out of Gas" error or to "Block Gas Limit". Need to create a mechanism for breaking cycles into parts.

Code snippet

1. moveFromStateAToStateB

2. setVisitsListTimestamps

3. moveFromStateAToStateB

4. visited

5. onlyAllowedMethods

3.24. Power Law (Rewards Distribution)

Severity: medium

Description

If a value of factor is higher than 2 in powerLawRewards function the cumulative reward for every address (numberOfInfluencers) will be higher than the total allocated reward totalRewardEthWEI.

Code snippet

https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/libraries/IncentiveModels.sol#L48

3.25. Contract Access Following Code Hash

Severity: high

Description

GetCode library function at access the code of another contract and load it into a bytes variable, this function is used in TwoKeyVoteToken to load contract code and calculate a hash that will be used to allow the contract or not to transfer tokens following the developers comments (check the code below).

    /// @notice function where admin or any authorized person (will be added if needed) can add more contracts to allow them call methods
    /// @param _contractAddress is actually the address of contract we'd like to allow
    /// @dev We first fetch bytes32 contract code and then update our mapping
    /// @dev only admin can call this or an authorized person
    function addContract(address _contractAddress) public onlyOwner {
        require(_contractAddress != address(0), 'addContract zero');
        bytes memory _contractCode = GetCode.at(_contractAddress);
        bytes32 cc = keccak256(abi.encodePacked(_contractCode));
        canEmit[cc] = true;
    }

    ///@notice Modifier which will only allow allowed contracts to transfer tokens
    function allowedContract() private view returns (bool) {
        //just to use contract code instead of msg.sender address
        bytes memory code = GetCode.at(msg.sender);
        bytes32 cc = keccak256(abi.encodePacked(code));
        return canEmit[cc];
        return true;
    }

    modifier onlyAllowedContracts {
        require(allowedContract(), 'onlyAllowedContracts');
        _;
    }

However when the code is loaded no state variable that has changed after the deployment is taken into account meaning that an attacker can load the code of a pre approved contract, deploy a new contract with the admin address changed (an example can be if the contract set the admin address inside the constructor), this will allow the attacker to access the same privileges as the original allowed contract on TwoKeyVoteToken or any inherited contract.

Pease note that no function is implemented to remove the privileges for a contract

Code snippet

https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/libraries/GetCode.sol#L10

https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/TwoKeyVoteToken.sol#L37

https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/TwoKeyVoteToken.sol#L46

https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/TwoKeyVoteToken.sol#L54

3.26. Compare Function

Severity: low

Description

compare function member of TwoKeyCongress compare only the first 3 bytes of the allowed function signature, if a not allowed function contain the same 3 first bytes the proposal can pass the validation phase.

Code snippet

https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyCongress.sol#L143

3.27. Maintainers Management

Severity: medium

Description

getAllMaintainers function implemented in of TwoKeyMaintainersRegistry and TwoKeyPlasmaMaintainersRegistry return an incomplete or a wrong array of maintainers addresses since it is only updated once when initialized but not updated when calling addMaintainer or removeMaintainer later on after initialization.

Code snippet

https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyMaintainersRegistry.sol#L104

https://github.com/RideSolo/contracts-4/blob/c85f3e1f3a04c56afd28bf673758367cc3df6609/contracts/2key/singleton-contracts/TwoKeyPlasmaMaintainersRegistry.sol#L79

4. Conclusion

The audited smart contract must not be deployed. Reported issues must be fixed prior to the usage of this contract.

5. Revealing audit reports

https://gist.github.com/yuriy77k/bf6a51d7397b343aa703670ce929d206

https://gist.github.com/yuriy77k/0af525ac76211e5810d9523e99991e01

https://gist.github.com/yuriy77k/279d6d0ed971b0a6ceedd1c639743075

https://gist.github.com/yuriy77k/3e57db7563e582b19942d923c500492f

5.1. Notes about MrCrambo report:

3.2. ERC-20 support isn't a security issue. The interface is used to define the functions of the target contract, which will be called from the current one. It is not intended to contain all the functions of the target contract.

3.8. TODO comments isn't a security issue. The contract is in development state, therefore it has TODO comments.

3.11. Truncated value isn't a security issue (except the last paragraph). All truncations are less than the minimum significant number. Only in the last paragraph the truncation may be less if use multiplication before division, but loss by this issue is too small (less than 1000 wei) - its severity "note".

5.2. Notes about danbogd report:

3.5. Transfer prevents transfers of zero value isn't a security issue. The TwoKeyCampaign contract is not a ERC20 token contract (it has no token name, decimal, transfer function, approve function, etc). It uses for manage other token campaign.

3.5. Transfer prevents spending all tokens isn't a security issue. This function will transfer 1 token from balances[_from] so it's right to check balance is greater of zero. It does not prevent spending all tokens.

5.3. Notes about gorbunovperm report:

3.1. Converter can block the rejection of himself isn't a security issue. The converter with ConverterState.PENDING_APPROVAL can not complete conversion, because in the function executeConversion there is require(converterToState[conversion.converter] == ConverterState.APPROVED);. Using described issue the harmful converter can always stay in PENDING_APPROVAL state and loss its contribution. It doesn't hurt smart contract.