TokenTrader.md

Ethereum Classic TokenTrader smart contract audit report.

Summary

This is the report from a security audit performed on TokenTrader smart contract (ETH) by alexo18. The audit focused primarily on the security of funds and fault tolerance of the TokenTrader contract. The contract serve as Decentralised Trustless Exchange Contract.

In scope

[TokenSellerFactory.sol]https://github.com/bokkypoobah/TokenTrader/blob/master/contracts/TokenSellerFactory.sol)
[TokenTraderFactory.sol]https://github.com/bokkypoobah/TokenTrader/blob/master/contracts/TokenTraderFactory.sol)

Findings

In total, 3 issues were reported including:

• 1 high severity issues.
• 2 medium severity issues.

Security issues

1. Risk of incorrect implementation of the 'transferFrom' asset contract function.

Severity: high

Description

At the moment the 'takerSellAsset' process relies only on the return value from the 'transferFrom' function of the asset issuer contract. Incorrect implementation of the 'transferFrom' by the asset issuer would cause the contract to lose funds.

Code snippet

https://github.com/bokkypoobah/TokenTrader/blob/master/contracts/TokenTraderFactory.sol#L317

Recommendation

Implement a mechanism which checks the contract balances before and after execution of the 'transferFrom' function.

2. Modifier pose a security risk.

Severity: medium

Description

The current implementation of the 'onlyOwnerOrTokenTraderWithSameOwner' modifier pose a security risk in that the third party will be able to deploy a contract which will be initialized with the same owner address as the real owner.

Code snippet

https://github.com/bokkypoobah/TokenTrader/blob/master/contracts/TokenTraderFactory.sol#L55

Recommendation

Provide a variable which represents the 'TokenTraderFactory' ,which creates the contract,and verify validity of the contract by using the 'TokenTraderFactory.verify(..' function.

3.Risk of sending money to a wrong address.

Severity: medium

Description

The current implementation of the makerTransferAsset, makerTransferEther functions pose a risk of sending money to the third party contract initialized with the same owner address as the real owner.

Code snippet

https://github.com/bokkypoobah/TokenTrader/blob/master/contracts/TokenSellerFactory.sol#L126 https://github.com/bokkypoobah/TokenTrader/blob/master/contracts/TokenTraderFactory.sol#L181 https://github.com/bokkypoobah/TokenTrader/blob/master/contracts/TokenTraderFactory.sol#L241

Recommendation

Verify validity of the target contract by using the 'TokenTraderFactory.verify(..' function.

Conclusion

Critical vulnerabilities were detected,we highly recommend to complete the bugs before the contract use.