Orcania security record

Due to the use of the non-standard ERC20 interface and the lack of checking the return value, attackers can steal a specific non-standard ERC20 token added by other honest users without depositing any of this specific non-standard ERC20 token themselves, however, it still requires to deposit `OCA` tokens. This attack can be applied in any non-standard ERC20 tokens, in which the function `transfer()` and `transferFrom()` fail without revert, i.e the functions only return `false`. Because the current code doesn't check the return value, it treats all transactions as successful if it does not revert. Therefore, all functions containing `IERC20(token).transferFrom()` statement without checking the return value and changed balance can be executed successfully, even if the ERC20 transaction fails. For example, [`ZRX` token](https://etherscan.io/address/0xe41d2489571d322189246dafa5ebde1f4699f498#code) is one of these non-standard ERC20 tokens. \ Here is a potential attack scenario that could take place: 1. An honest user Alice sets liquidity with a `ZRX-OCA` pair using the function `setLiquidity()`. She deposits 200 `ZRX` tokens and 100 `OCA` tokens, and both tokens' transactions succeeded. She can earn `(OCAamount * 9999) / 10000 = 100 * 9999 / 10000 = 99` points. 2. A malicious user Bob then adds liquidity to the `ZRX-OCA` pair by calling the function `addLiquidity()`. He set the `amount` to 100 but does not have enough `ZRX` tokens, the transfer fails but the `transferFrom()` function only returns `false`. However, the current code does not check the return value and treats it as a successful transaction. Bob is believed to have added 100 `ZRX` tokens and 50 `OCA` tokens, so he earned `(totalPoints * amount / contractTokenBalance) * 9999 / 10000 = (100 * 100 / 200) * 9999 / 10000 = 49` points without transferring any `ZRX` tokens to the contract. 3. Bob then withdraws `ZRX` tokens by calling the function `withdrawLiquidity()`, because he earned 49 points, he can withdraw `IERC20(ZRX).balanceOf(address(this)) * points / totalPoints = 200 * 49 / 150 = 65` `ZRX` tokens from 200 `ZRX` tokens added by Alice.

Due to the use of the non-standard IERC20, and lack of checking on the return value, an attacker can set liquidity with a fake ERC20, and then use this fake token to swap for OCA and other legit ERC20 tokens. In addition, this action may also trigger the underflow on `_tokenOCAbalance[token]` when swapping. Here is a scenario to explain this attack. 1. A hacker Alice creates an ERC20 token `FAKE` specific for this attack, in which the function `transferFrom()` and `transfer()` do not do anything, and view function `balanceOf()` always returns 1 (note: 1 is an example and the attack does not rely on these specific values). 2. Alice sets liquidity with a `FAKE-OCA` pair by calling the function `setLiquidity()`. She sets the `amount` to 100 and `OCAamount` to 1. Because the function `transferFrom()` in `FAKE` token does nothing, none of the FAKE tokens will be transferred to the current contract, while the one `OCA` token is transferred successfully, and now the variable `_tokenOCAbalance[FAKE]` equals to 1. 3. Alice then calls the external function `swapTokenForOCA()` with `tokenIn` as `FAKE` token and `amountIn` as a very large value. Similarly, the function `IERC20(FAKE).transferFrom()` does not transfer any `FAKE` tokens to the current contract. 4. Then, in the function `SwapTokenForOCA()`, the amount of `OCA` token sent to the user will be equal to `amountOut = amountIn * _tokenOCAbalance[token] * 999) / IERC20(token).balanceOf(address(this)) * 1000) = amountIn * 999 / 1000`. If `amountIn` is greater than 1, the value of `amountOut` is strictly greater than 1. Once executing the following statement `_tokenOCAbalance[token] -= amountOut`, an underflow issue happens and `_tokenOCAbalance[FAKE]` becomes extremely high. 5. Because the value of `_tokenOCAbalance[FAKE]` is large enough, Alice can call the external function `swapTokenForOCA()` again with a crafted `amountIn` and withdraw all `OCA` tokens in the contract which are deposited with other pairs. 6. After withdrawing enough `OCA` tokens, Alice can use those `OCA` tokens to swap for other valuable legit ERC20 tokens deposited in this DEX by calling the function `swapOCAForToken()`.

The contract `DEX` provides the functionality that set liquidity paired with OCA and swaps tokens via OCA. However, due to the lack of checking the token address to be added as liquidity or swapped for, a malicious user can set an OCA-OCA paired liquidity and swap for OCA by any function that calls the internal function `SwapOCAForToken()`. In both cases, the calculations of withdrawing and swapping are wrong. Specifically, if a user set an OCA-OCA paired liquidity by the function `setLiquidity(token:OCA, amount:200, OCAamount:100)`, he will get `99` points. After that, he withdraws 10 points, which should equal to 20 tokens(OCAs) and 10 OCAs normally, but he actually can get 30 tokens(OCAs) and 10 OCAs because the withdrawn token amount is calculated from `IERC20(token).balanceOf(address(this))`, which is all OCA deposited in the contract. ```solidity=192 uint256 tokenAmount = (IERC20(token).balanceOf(address(this)) * points) / totalPoints; ``` As for the swapping scenario, suppose the contract now has 100 OCAs and has no liquidity pool of OCA-OCA pair, i.e., `IERC20(OCA).balanceOf(address(this)) == 100` and `_tokenOCAbalance[token] == 0`. If an user swaps 1 OCA for OCA by the function `swapOCAForToken()`, he can gain `99(=1*100*999/((0+1)*1000))` OCAs. Then he can use these 99 OCAs to swap for other valuable tokens. ```solidity=311 amountOut = (amountIn * IERC20(token).balanceOf(address(this)) * 999) / ((_tokenOCAbalance[token] += amountIn) * 1000); ```

The contract `OCA.sol` uses a version of solidity strictly anterior to 0.8.0, hence there are no built-in checks in the compiler for overflow/underflow. This could be used by an attacker, for example: 1. the hackers uses `faucetMint()` to have a `balance > _totalSupply` 2. the hacker burns an `amount` > `_totalSupply` and because underflow is not prevented: `_totalSupply` can become "huge" and especially greater than 250000000000000000000000001. 3. calling `mint()` will: - revert, if `_totalSupply += amount` does not overflow, because `_totalsupply` is always greater than 250000000000000000000000001 - not revert, if `_totalSupply += amount` overflows, then `_totalSupply` becomes very small and the amount will be minted if it is less than 250000000000000000000000000. As a result, the value of the `balances` and the `_totalSupply` cannot be trusted and the whole contract is not reliable anymore. ----- The contracts inside the scope of the audit use a lot of arithmetic operators which could overflow/underflow. This presents a systemic risk because a single point of failure as shown above could make the contracts completely unreliable.

In the contract `OCA` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and change name and symbol.  In the contract `OMS` the role `_manager` has authority over the functions shown in the diagram below. Since contract `OCA` inherits contract `OMS`, any compromise to the `_manager` account may allow the hacker to take advantage of this authority and withdraw ETHs and ERC20 tokens.  In the contract `OMS` the role `_owner` has authority over the functions shown in the diagram below. Since contract `OCA` inherits contract `OMS`, any compromise to the `_owner` account may allow the hacker to take advantage of this authority and set owner and manager. 

Incorrect return values for ERC-20 functions. A contract compiled with Solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing. IERC20 (DEX/DEX.sol#4-10) has incorrect ERC20 function interface: IERC20.transfer(address,uint256) (DEX/DEX.sol#7) ```solidity=7 function transfer(address recipient, uint256 amount) external; ``` --- IERC20 (DEX/DEX.sol#4-10) has incorrect ERC20 function interface: IERC20.transferFrom(address,address,uint256) (DEX/DEX.sol#9) ```solidity=9 function transferFrom(address sender, address recipient, uint256 amount) external; ```

There are two important issues with the function `faucetMint()` in the `OCA.sol` contract: 1. No permission check for function `faucetMint()`, so anyone can call it to increase his/her balance with a fixed value. 2. No update on `_totalSupply` after updating `_balances` and no check if `_totalSupply` is greater than the set limit `250000000000000000000000001`.

No check if the length of the two arrays `recipient` and `amount` are the same in Function `multipleTransfer`.

Missing check the return value of function `OCA.transferFrom` and `OCA.transfer`.

There should always be events emitted in the sensitive functions that are controlled by centralization roles.