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

Weidex v2 #243

Closed yuriy77k closed 5 years ago

yuriy77k commented 5 years ago

Audit request

Decentralized exchange for crypto assets. Improved version of the protocol + built-in incentive for the makers.

Source code

https://github.com/weichain/weidex-eth-v2

Disclosure policy

support@weidex.market

Platform

ETH

Number of lines:

904 * 0.5 = 452 (reaudit https://github.com/EthereumCommonwealth/Auditing/issues/84 )

MrCrambo commented 5 years ago

Auditing time 3 days

yuriy77k commented 5 years ago

@MrCrambo assigned

RideSolo commented 5 years ago

Estimated auditing time: 3 days.

yuriy77k commented 5 years ago

@RideSolo assigned

mobilipia commented 5 years ago

Estimated auditing time 3 days

yuriy77k commented 5 years ago

@mobilipia assigned

mobilipia commented 5 years ago

Cant find the audit manager's email address

yuriy77k commented 5 years ago

@mobilipia please, send your report to yuri@callisto.network

yuriy77k commented 5 years ago

WeiDex v2 Security Audit Report

1. Summary

WeiDex v2 smart contract security audit report performed by Callisto Security Audit Department

2. In scope

3. Findings

In total, 7 issues were reported including:

No critical security issues were found.

3.1. Referral address require checking for zero

Severity: low

Description

Referrals addresses are set in deposit function member of ExchangeMovements contract, if the users do not input a referral address and leave it empty, the referral reward will be assigned to address(0) in executeTrade function member of Exchange contract.

The impact will be locking an amount of different tokens to address 0x0 without possibility of withdrawal, the amount can vary following the traded volume and the number of users without referral addresses.

Code snippet

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/ExchangeMovements.sol#L69#L72

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/Exchange.sol#L242

Recommendation

Check referrer address in executeTrade where referrer should be different than address(0) and allocate the referral reward following the result.

3.2. Exchange Upgrade

Severity: low

Description

importEthers/importTokens function member of ExchangeUpgradability do not set the referral address for a user when importing the user fund from an old exchange address. this issue will cause the same problem described in " Referral Reward" issue.

Code snippet

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/ExchangeUpgradability.sol#L116

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/ExchangeUpgradability.sol#L146

3.3. Exchange Balance Transfer

Severity: low

Description

In transfer function member of ExchangeMovements contract some requirement should be set to avoid sending balances to wrong addresses.

Code snippet

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/ExchangeMovements.sol#L119

Recommendation

Add the following lines to the function:

    require(to!=address(0));
    require(to!=address(this));

3.4. Exchange Balance Transfer

Severity: low

Description

migrateFunds function member of ExchangeUpgradability does not check if the new exchange address is set to non null address.

Code snippet

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/ExchangeUpgradability.sol#L58

3.5. ERC-20 Issues

Severity: notes

Description

The following issues are part of mock files that are probably used for test only:

Code snippet

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/mocks/OldERC20.sol

3.6. Experimental Features

Severity: notes

Description

As raised by the compiler "Experimental features are turned on. Do not use experimental features on live deployments" the audited code uses ABIEncoderV2 that is in experimental phase and should not be deployed in a live network.

Code snippet

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/Exchange.sol#L2

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/ExchangeBatchTrade.sol#L2

https://github.com/RideSolo/weidex-eth-v2/blob/master/contracts/exchange/ExchangeOffering.sol#L2

3.7. Owner privileges

Severity: owner privileges

Description

Owner can migrate exchange to new contract address. It may has issues if was not audited.

4. Conclusion

The audited smart contract can be deployed. Only low severity issues were found during the audit.

5. Revealing audit reports

https://gist.github.com/yuriy77k/95510c49110e25766c1d75bd99e8d307

https://gist.github.com/yuriy77k/0523a14212bf02bdd9a6c7a047eaa002

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

MillianoConti commented 5 years ago

Announced:https://www.reddit.com/r/CryptoCurrency/comments/c4ktxo/security_audit_of_weidex_performed_by_callisto/