Technical

IDEX Exchange Smart Contract Audit

IDEX is a semi-decentralised exchange built on top of Ethereum enabling trading of ether and ERC20 tokens. It is built in such a way that “the smart contract is used for custody of funds, trade, and settlement, while the IDEX servers manage the off-chain balances, orderbook, and trade dispatching”.

IDEX exchange smart contract is availabe at https://github.com/AuroraDAO/idex-api-docs/blob/master/Exchange.sol commit (d007b513e3e615e2725b937dd259893053903078) and is deployed on the mainnet here.

This document describes an independent smart contract analysis.

Introduction

First of all, we applaud IDEX for a well architected solution and intelligent application of digital signing to create a successful scalable exchange.
Secondly, as a way of setting background to this analysis, let’s have a look at a typical scenario of two users interacting with the exchange:

  • Alice deposits ether to IDEX using deposit method
  • Bob wants to deposit some ERC20 tokens:
  • Bob gives the allowance to IDEX by calling ERC20.approve
  • Bob calls depositToken method on IDEX
  • Alice as a market maker signs a create order message to buy ERC20 tokens with ether and sends it to IDEX
  • IDEX backend validates the message and displays the order on the website
  • Bob as a market taker wants to fill Alice’s order, so he signs the trade message and sends it to IDEX
  • IDEX backend matches the trade and calls trade method which swaps the balance of ether and ERC20 for Alice and Bob and, at the same time, takes the fees from both
  • Alice wants to withdraw her new tokens so she signs a withdraw request and sends it to IDEX
  • IDEX processes the requests and calls adminWithdraw on Alice’s behalf and takes the withdrawal fee.

Analysis

This analysis is divided in two distinct aspects: user/system trust and the smart contract code itself. First aspect touches on the amount of trust that users have to place on IDEX and if there is anything that could go wrong. And the second one is a traditional code level audit. We concluded the analysis and identified several issues in both of the categories which are described in more detail below. Let’s look at them in turns and present our findings.

Trust

There are two on-chain operations which IDEX performs on user’s behalf: trade execution and withdrawals.

Users request trades by signing messages and forwarding them to IDEX backend and it’s up to IDEX to execute them. As a result, there is no time frame or any guarantee at all that the trade will be executed. Having said that, it is in IDEX’s best interest to perform as many trades and as quickly as possible to maximise their commission.

This leads us to fee management where we believe we have identified a potential trust issue in the way that the commissions are applied. According to FAQ: “IDEX charges 0.2% for the market taker and 0.1% for the market maker” in addition to the gas cost of the transaction covered by the taker: “Users also pay gas fees to put their transactions on blockchain”.

However, digging into the code and more specifically the trade method, we discovered that it is possible for IDEX to charge both market maker and taker up to 10% of the trade ether/tokens in commissions. It is defined in those two lines of code:

if (tradeValues[6] > 100 finney) tradeValues[6] = 100 finney;
if (tradeValues[7] > 100 finney) tradeValues[7] = 100 finney;

where tradeValues[6] and tradeValues[7] represent the amount of ether/tokens of market maker and market taker correspondingly. In the context of this smart contract, 1 ether is defined as 100%, therefore above value of 100 finney (0.1 ether) represents 10%.

Note, we have not seen this happen, nor believe that IDEX would ever start charging 10% in fees, but it is a possibility from the code point of view.

Another time when users put trust on IDEX is during the withdrawals of funds from the exchange smart contract. It is designed in such a way to prevent users making the withdrawals of funds already allocated to some trades. As in the previous case, users sign withdrawal requests and forward them to IDEX for execution (calling adminWithdraw method). As IDEX pays the gas cost of the transaction, and once again they claim the cost back from the market taker. The limit is defined in code as 5% of the value.

if (feeWithdrawal > 50 finney) feeWithdrawal = 50 finney;

Our recommendation would be to lower the fee limits closer to the actual cost, particularly for market maker, but also for market taker and withdrawals taking into consideration the historical analysis of gas prices.

On a side note, there is a risk that user deposited funds get locked in the smart contract if IDEX stops processing the withdrawals. It is however mitigated with the withdraw functions which can be executed directly by users when inactivityReleasePeriod expires. For reference, the funds could be locked up to around 17.5 days.

Code

No critical issues have been found during the code audit. We have identified several minor issues which are described below.

1. Surplus Event Definitions

There are three declared events Order, Cancel and Trade which are not emitted anywhere. There is no logical place to emit the first two, as both order and cancel operations are achieved through message signing and not in the smart contract. It is therefore recommended to remove them.

As far as the Trade event is concerned, we would recommend to emit it at the end of the trade function.

2. Deprecated Throws

IDEX smart contract is using Solidity compiler version 0.4.16, but it has multiple throw statements which have been deprecated since 0.4.10.
There is also a custom definition of assert which overrides the one provided by Solidity off the shelf:

function assert(bool assertion) {
if (!assertion) throw;
}

It is recommended to remove the above method and replace throw with require, revert and assert checks.
Another deprecated method send used for transferring ether is used in the below statement:

if (!user.send(amount)) throw;

It is recommended to replace it with transfer method.

3. Missing Return Variables

There are three methods `withdraw`, `adminWithdraw` and `trade` which define returns (bool success) but don’t actually define the success variable nor explicitly return anything from the method. The recommendation is therefore to remove the returns statements.

4. Code Layout

The overall organisation of the code could be also improved as it contains intermixed declarations of variables, modifiers and functions and is difficult to read. It seems that separate contracts (equivalents of SafeMath and Ownable) where stripped out the contract keyword and thrown all into one Exchange contract.

As a side note, some methods, like trade change the values of the input arguments which is not a recommended practice, but it is required in this case as declaring any additional local variables hits a current Ethereum limitation of: “stack too deep remove local variable”.

5. Surplus Code

IDEX contract declares a default fallback function and makes it fail with the intention of refusing ether deposits. This is not necessary as since Solidity 0.4.0 contracts without default fallback automatically revert payments, so this function definition could be removed.

Summary

In conclusion, we have not found any critical issues with IDEX exchange smart contract, but have identified several minor ones described above. We would recommend to address those and at the same time upgrade to the latest Solidity compiler which is 0.4.24 at the time of writing as it includes several improvements.

Next Article
1 Star2 Stars3 Stars4 Stars5 Stars (2 votes, average: 5.00 out of 5)
Loading...
Radek Ostrowski
Blockchain Engineer and open source contributor, particularly interested in Ethereum and Smart Contracts. In the fiat world, experienced in Big Data/Machine Learning projects. Co-creator of PlayStation 4 backend. Successful hackathon competitor.

1 Comment

Leave a Reply

Your email address will not be published. Required fields are marked *