carlossampol commented 5 years ago

Audit request

WeiDex is decentralized exchange based on Ethereum

Our code have been audited https://github.com/EthereumCommonwealth/Auditing/issues/243.

We have added new functionality that connects our contracts to Kyber network contracts.

We would like to donate 200$ dollars in ETH, because we are extremely happy with the services that you are providing and we want to support the network.

You could send me your donation address via email.

Source code

https://github.com/weichain/weidex-eth-v2/tree/master/contracts/exchange

Disclosure policy

titov@weidex.market

Platform

Ethereum

Number of lines:

488 (976 * 0.5 reaudit coefficient)

MrCrambo commented 5 years ago

Auditing time 4 days

yuriy77k commented 5 years ago

@MrCrambo assigned

MrCrambo commented 5 years ago

My report is finished

danbogd commented 5 years ago

Auditing time: 3 days.

yuriy77k commented 5 years ago

@danbogd assigned

danbogd commented 5 years ago

My report is finished.

RideSolo commented 5 years ago

@yuriy77k I have few remarks before that I can take this audit, reaudit coefficient is when the audit is reposted with old issues solved however here new functionality are added, why the reaudit coeficient applied to the new files also ? follwing the number of line posted, utils folder was excluded from the audit or at least by mistake, however new utils files have been added and have to be audited also, other wise the audit cannot be concluded. Can you please add the in scop files to the audit request ?

yuriy77k commented 5 years ago

@RideSolo reaudit coefficient used to indicate average changes in code. Do you propose to calclulate only changed lines and pay for it?

Utils folder was excluded because contains only common and simple functions.

RideSolo commented 5 years ago

@yuriy77k,I do not propose anything I'm asking to make the point clear for future audit request. for utils folder, simpler means fewer line of code, and we cannot assume that the code is a common code untill the audit is conducted, meaning that it has to be audited anyway.

RideSolo commented 5 years ago

Auditing time: 3 days.

yuriy77k commented 5 years ago

@RideSolo assigned

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

Commit hash 15329af7964f15fa9ff5884f144c69cb6343b620.

3. Findings

In total, 7 issues were reported including:

1 medium severity issues.
4 low severity issues.
1 notes.
1 owner privileges (the ability of an owner to manipulate contract, may be risky for investors).

No critical security issues were found.

3.1. Referral Reward (Updated)

Severity: medium

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 or ether to address 0x0 without possibility of withdrawal, the amount can vary following the traded volume and the number of users without referral addresses.

Three scenarios can present themselves:

The user which is the msg.sender is different than the benificiary since the benificiary can be selected by the user when making the deposit, in this case all referrals fees generated by the benificiary will be sent to 0x0 address , please note that the UI cannot solve this issue, since there is a inputted referral address but it is set for the user not the beneficiary.
The second scenario is if the user is also the benificiary, meaning that if a referral address is inputted it will be set for the benificiary which will be equal to user and msg.sender.
The third scenario is if the referral address is set to zero, or not set at all (If the users use the UI, this issue can be solved).

Please note that referral address is set to the user not to the benificiary and these two addresses can be different.

Code snippet

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeMovements.sol#L52

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeMovements.sol#L59

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeMovements.sol#L67

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeMovements.sol#L70

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/Exchange.sol#L276

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/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeUpgradability.sol#L116

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/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/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeMovements.sol#L119

Recommendation

Add the following lines to the function:

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

3.4. 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/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/Exchange.sol#L2

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeBatchTrade.sol#L2

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeOffering.sol#L2

3.5. Length Comparison

Severity: low

Description

Compare orders and signatures lengths before the for loop.

Code snippet

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeBatchTrade.sol#L11

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeBatchTrade.sol#L29

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/ExchangeBatchTrade.sol#L45

3.6. Contract accept payment from anyone.

Severity: low

Description

An anybody, who send Ether to contract address may lose it because of no payment processing in contract code.

Code snippet

https://github.com/weichain/weidex-eth-v2/blob/15329af7964f15fa9ff5884f144c69cb6343b620/contracts/exchange/WeiDex.sol#L21

3.7. Owner Privileges

Severity: owner privileges

Description

Contract owner allow himself to:

Owner can 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 here.

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/0482b9898def04cbf6f08f95a461945c

https://gist.github.com/yuriy77k/992a3295a23deaa3c31de4b6958b3ebb

https://gist.github.com/yuriy77k/70f972ce255fad113c1b19e00dd708f5

EthereumCommonwealth / Auditing

Weidex V2. #300

Audit request

Source code

Disclosure policy

Platform

Number of lines:

Weidex V2 Security Audit Report

1. Summary

2. In scope

3. Findings

3.1. Referral Reward (Updated)

Severity: medium

Description

Code snippet

Recommendation

3.2. Exchange Upgrade

Severity: low

Description

Code snippet

3.3. Exchange Balance Transfer

Severity: low

Description

Code snippet

Recommendation

3.4. Experimental Features

Severity: notes

Description

Code snippet

3.5. Length Comparison

Severity: low

Description

Code snippet

3.6. Contract accept payment from anyone.

Severity: low

Description

Code snippet

3.7. Owner Privileges

Severity: owner privileges

Description

4. Conclusion

5. Revealing audit reports