Airswap

The function `Delegate.setRuleAndIntent` sets the stake of the delegate owner in the `Indexer` contract by transferring staking tokens from the user to the indexer, through the `Delegate` contract. However, if the function is called a second time, the underlying `Indexer.setIntent` will only transfer a partial amount of the total transferred tokens (i.e., the delta of the previously set intent value versus the currently set intent value). Since the behavior of the `Delegate` and the `Indexer` differ in this regard, tokens can become stuck in the Delegate. This is elaborated upon in issue [274](https://github.com/airswap/airswap-protocols/issues/274).

Smart contracts will often have `owner` variables to designate the person with special privileges to make modifications to the smart contract. However, this centralization of power needs to be made clear to the users, especially depending on the level of privilege the contract allows to the owner. In particular, the owner may `selfdestruct` the contract locking funds forever. Since the `killContract()` function does not send any of the transferred tokens out of the contract, all tokens will remain permanently locked in the contract if not previously removed by users. The platform can censor transaction via `unsetIntentForUser()`: whenever an intent is set, the owner can just unset it. It is also not easy to see if it is the owners who did this, as the event emitted is the same. The owner may also permanently pause the contract locking funds.

On L233 and L290, we have the following two conditions that relate sender and signer order amounts: * L233 (Equation "A"): `order.sender.param == order.signer.param.mul(10 ** rule.priceExp).div(rule.priceCoef)` * L290 (Equation "B"): `signerParam = senderParam.mul(rule.priceCoef).div(10 ** rule.priceExp);` Due to integer arithmetic truncation issues, these two equations may not relate as expected. Consider the case where: * rule.priceExp = 2 * rule.priceCoef = 3 For Equation B, when senderParam = 90, we obtain `signerParam = 90 * 3 // 100 = 2` due to integer truncation. However, when plugging back into Equation A, we obtain `2 * 100 // 3 = 66` (which doesn't equal the expected value of 90`. This means that if the signerParam is calculated by the logic in Equation B, it would not pass the requirement of Equation A.

While the `Swap` contract ensures that an `Order` is correctly formatted and properly signed in the `isValid()` function, it does not check that the intended `Swap` contract, as denoted by the `Order.signature.validator` field, corresponds to the correct `Swap` contract. If a sender/signer participates with multiple swap contracts, replay attacks may be possible by re-submitting the order.

On L349: `require(initialBalance.sub(param) == INRERC20(token).balanceOf(from), "TRANSFER_FAILED");` is a dangerous equality. Although this will hold for most ERC20 tokens, the specification does not guarantee that the external ERC20 contract will adhere to this condition. For example, the token could mint or burn tokens upon a `transfer()` for various reasons.

On L346 of `Swap.sol`, `ERC20.transferFrom()` is expected to return a boolean value indicating success. Although the `INRERC20` is used here, the underlying contract is most likely an `ERC20` token, and therefore its return value should be checked for success.

Most functions will return a `true` or `false` value upon success. Some functions, like `send()`, are more crucial to check than others. It's important to ensure that every necessary function is checked. On L151 of `Wrapper`, the external `call.value()` is not checked for success, which may cause ether transfers to the user to fail. A similar issue exists for the `transfer()` on L127 of `Wrapper`, and L340 of `Swap`.