EthereumCommonwealth / Auditing

Ethereum Commonwealth Security Department conducted over 400 security audits since 2018. Not even a single contract that we audited was hacked. You can access our audit reports in the ISSUES of this repo. We are accepting new audit requests.
https://audits.callisto.network/
GNU General Public License v3.0
131 stars 34 forks source link

EZOToken v2 #427

Closed yuriy77k closed 4 years ago

yuriy77k commented 4 years ago

Audit request

Element Zero - Smart Swap Contract. Documentation attached.

Element Zero - Smart Swap Blockchain Document (4).docx

Source code

https://github.com/ezo-network/ezo-token/tree/master/ezotoken/contracts

Disclosure policy

Standard disclosure policy.

Contact information (optional)

kyle@jointer.io

Platform

ETH

Budget

1.995 Ether Reaudit of https://github.com/EthereumCommonwealth/Auditing/issues/422#issuecomment-582556085

danbogd commented 4 years ago

Auditing time: 5 days (excluding weekends).

MrCrambo commented 4 years ago

Auditing time 6 days.

yuriy77k commented 4 years ago

@danbogd @MrCrambo assigned

RideSolo commented 4 years ago

Estimated audit time: 6 days

yuriy77k commented 4 years ago

@RideSolo assigned

gorbunovperm commented 4 years ago

Estimated auditing time is 6 days.

yuriy77k commented 4 years ago

@gorbunovperm assigned

danbogd commented 4 years ago

My report is finished.

KDubCrypto commented 4 years ago

Is there an update here?

RideSolo commented 4 years ago

@KDubCrypto, what do you intend to use SecureETH contract for ?

KDubCrypto commented 4 years ago

@KDubCrypto, what do you intend to use SecureETH contract for ?

@RideSolo to allow users to freeze the value of their Ether instantly. So in volatile times, they hit freeze / secure contract and the value stays.

KDubCrypto commented 4 years ago

@yuriy77k what is going on here? Very delayed

KDubCrypto commented 4 years ago

@yuriy77k can we get a refund? We had to engage another auditing firm 0x084374b068Eb3db504178b4909eDC26D01226a80

yuriy77k commented 4 years ago

I apologize for the delay in auditing, but due to the COVID-19 situation, some of the auditors toke more time for auditing then expected.

yuriy77k commented 4 years ago

The contract contains high severity security issues. The developer is informed about it.

yuriy77k commented 4 years ago

EZO Token V2 Security Audit Report

1. Summary

EZO Token V2 smart contract security audit report performed by Callisto Security Audit Department

2. In scope

Commit hash ee918152d177f8dcee02a76431bdace3b5647878.

3. Findings

In total, 16 issues were reported including:

3.1 SmartSwap & SecureETH mint

Severity: high

Description

Following the description provided by the team about the usage of SecureETH contract:

The implementation of SecureETH does not protect users from volatility in any way:

Code snippet

https://github.com/ezo-network/ezo-token/blob/ee918152d177f8dcee02a76431bdace3b5647878/ezotoken/contracts/SmartSwap.sol#L327#L334

https://github.com/ezo-network/ezo-token/blob/ee918152d177f8dcee02a76431bdace3b5647878/ezotoken/contracts/SmartSwap.sol#L336#L353

https://github.com/ezo-network/ezo-token/blob/ee918152d177f8dcee02a76431bdace3b5647878/ezotoken/contracts/SmartSwap.sol#L362

3.2 Transfer Computation

Severity: high

Description

Both returnAmount and sentAmount calculation assumes that the EZO price is fixed to 100 USD, however CurrencyPrices(currencyPricesContract).currencyPrices(address(this)) was used when computing [_valueCal](), all calculation should reflect the same price.

3.3 Smart Swap Deposit

Severity: high

Description

When using smart swap contract deposit through sendEther or sendToken an order is automatically added (addOrder) and fulfilled ([generalFundAssign](), generalFundAssignEZO), if the wanted currency balance in the smart-swap contract is higher than what the user is requesting and the deposited currency is ezo tokens.

A user that deposited tokens or ether previously might not be able to withdraw his deposit since it can be swapped with other users deposit that deposited ezo tokens even if his wanted currency and sent currency are different than ezo tokens, check here

Users that deposit tokens to swap them against EZO will be automatically accredited newly minted tokens to their account, the deposited tokens will be kept inside the contract, check here

This logic need a balanced deposit between all tokens otherwise some users orders might not be fulfilled and and their deposit might be spent, blocking them from using cancelOrder.

3.4 Rate Conversion

Severity: high

Description

The conversion operation in getCalValue is wrong:

Recommendation

    function getCalValue(uint256 returnCurrencyAmount, uint256 remainingAmountNewOrder,address _currencySent,address _currencyWant) internal view returns(uint256)
    {
        return safeDiv(
            safeMul(
                safeDiv(
                    safeMul(
                        safeSub(returnCurrencyAmount,remainingAmountNewOrder), 
                        10**CurrencyPrices(currencyPricesContract).currencyDecimal(_currencySent), 
                    10**CurrencyPrices(currencyPricesContract).currencyDecimal(_currencyWant), 
                CurrencyPrices(currencyPricesContract).currencyPrices(_currencySent)
                , 
            CurrencyPrices(currencyPricesContract).currencyPrices(_currencyWant)
        );
    }

3.5 SecureETH Gas Limit

Severity: high

Description

burn function implemented in SecureETH requires the user address to be whitelisted in allowedForBurningTokens. allowedForBurningTokens is an array and to check if a user is whitelisted isAllowed modifier iterates over the array, when a simple mapping can be used to get the value directly.

This issue will cause high gas consumption, until the contract will become unusable (following the array length).

Code snippet

https://github.com/ezo-network/ezo-token/blob/ee918152d177f8dcee02a76431bdace3b5647878/ezotoken/contracts/secureETH.sol#L163#L182

3.6 EZO Deposit

Severity: medium

Description

When cancelling an order generalFundAssign is used to refund the user tokens or ether, if the deposited tokens through sendToken are EZOs, instead of sending the tokens back to the user using assignTokens with the sender address equal to the EZO contract address, the developers used mint function which will create new tokens. The deposited EZO tokens will be frozen inside the contract making the token supply higher, an attacker can repeatedly deposit/cancel to make the total supply higher just to hurt the project. Please note that no max cap is setting when minting.

3.7 Transfer Return Value

Severity: medium

Description

When _uniqueId is equal to systemAddress the return value of the transfer function should be set to true, since it is a valid transaction, however the function will return false. Developers must simply add a return statement here.

3.8 ERC 20 Compliance

Severity: medium

Description

Depending on the intention of the developers, the transfer function is not ERC20 compatible and users won't be able to transfer EZO tokens following the normal ERC20 rules.

3.9 ERC20 Compliance — method missing

Severity: medium

Description

In the ERC-20 standard should be approve, transferFrom functions and event Approval, but here they are missing.

3.10 SecureETH ERC20 Compliance

Severity: low

Description

SecureETH contract inherit from IERC20 interface but do not implement the ERC-20 functions following the standard, the developers should define the usage of such contract to allow us to conclude the risk related with it.

Code snippet

https://github.com/ezo-network/ezo-token/blob/ee918152d177f8dcee02a76431bdace3b5647878/ezotoken/contracts/secureETH.sol

3.11 Known vulnerabilities of ERC-20 token

Severity: low

Description

  1. It is possible to double withdrawal attack. More details here
  2. 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

3.12 Owner Privileges

Severity: owner privileges

Description

Please note that most function marked with "onlyOwner" can either remove trust that the blockchain technology enforce between users and developers or can be hacked in case if the private key is stolen.

  1. setCurrencyPriceUSD allow the contract owner to set any currency value, instead decentralized oracle can be used such as "chainlink".
  2. Owner can whitelist any address allowing it to burn and mint tokens from any address using addAllowedAddress
  3. Owner can updateTxStatus and block the user fund for a specific transaction.
  4. The owner can prohibit the transfer of tokens by halt() function.
  5. Owner can change currency smart contract address and EZO token smart contract address at any time and without any restrictions here, here and here.

4. Conclusion

The audited contracts are unsafe and should not be deployed due to multiple high severity. User funds are at risk.

5. Revealing audit reports

https://gist.github.com/RideSolo/93fa7a343dc8b7e3c05ed969de963a5a

https://gist.github.com/gorbunovperm/832a41de5bccfbb8e471e64fd102dd41

https://gist.github.com/MrCrambo/e49da6f45c403c2cb072499b65b08ac2

https://gist.github.com/danbogd/13b8cdefbdbefbcb19a94090cf249f68