Tokemak

The core flow of the system (i.e., supplying liquidity) is currently carried out by the admin team in an off-chain fashion. Precisely put, the calculations of slashing, reward distributions, and asset allocations take place of the chain. Based upon those calculations, the manager submits the transactions to the blockchain. This design assumes a high trust in the manager. The manager can be compromised or go rogue in which case users can lose funds.

An attacker can obtain staking power if there is any `schedule` with a passed `hardStart`. The code never checks if funds are being deposited to an already expired `schedule`. Moreover, the code does not unset the `isActive` flag for expired schedules. This could result in a malicious user staking a large amount of funds in order to vote for their favorable target controller followed with an immediate withdraw of their funds. This scenario could be used as a utility to conduct a more complex attack.

Upon addition (`_addSchedule`) of schedules the setup property is set to true. The require statements in the `slash` and `depositFor` functions seem to be useless.

As it presently is constructed, the contract is vulnerable to the [allowance double-spend exploit](https://github.com/OpenZeppelin/openzeppelin-solidity/blob/b4f87bb8fc25fb07f73099701e39e167a3d36465/contracts/token/ERC20/ERC20.sol#L71-L78), as with other ERC20 tokens.

The visibility of function `deploy` and `withdraw` are set to `external` and without any permission check. With this, an attacker can withdraw all the funds that are in the other pools first, then drain the funds returned to the contract by deploying funds into a malicious pool under the attacker’s control. Right now, the manager uses the vulnerable controller through delegate calls. Therefore, the manager’s storage is used for the platform funds. If someone directly calls the controller it would net no funds.

Some loops are not capped at a max number of supported iterations. This has certain implications on the gas usage. > Gas usage is a main concern for smart contract developers and users, since high gas costs may prevent users from wanting to use the smart contract. Even worse, some gas usage issues may prevent the contract from providing services entirely. For example, if a `for` loop requires too much gas to exit, then it may prevent the contract from functioning correctly entirely. It is best to break such loops into individual functions as possible. Listed are the loops that we deem as unsafe with respect to gas usage: 1. `contracts/manager/Manager.sol:executeMaintenance:L125` function: ```solidity=125 for (uint256 x = 0; x < params.cycleSteps.length; x++) ``` 2. `contracts/manager/Manager.sol:executeRollover:L147` function: ```solidity=147 for (uint256 x = 0; x < params.cycleSteps.length; x++) { _executeControllerCommand(params.cycleSteps[x]); } ``` 3. `contracts/manager/Manager.sol:L152` ```solidity=152 for (uint256 y = 0; y < params.poolsForWithdraw.length; y++) ``` 4. contracts/staking/Staking.sol:102~104: ```solidity=102 for(uint256 i = 0; i < length; i++) { retSchedules[i] = StakingScheduleInfo(schedules[scheduleIdxs.at(i)], scheduleIdxs.at(i)); } ```

Smart contracts will often have `owner` variables to designate the person with special privileges to make modifications to the smart contract. The owner can pause contracts and perform privileged operations. For instance, the owner can withdraw the diff of already withdrawn and initially vested tokens to the treasury. Users can lose funds if the owner account gets compromised or they go rogue.

> A common attack in DeFi is the sandwich attack. Upon observing a trade of asset X for asset Y, an attacker frontruns the victim trade by also buying asset Y, lets the victim execute the trade, and then backruns (executes after) the victim by trading back the amount gained in the first trade. Intuitively, one uses the knowledge that someone’s going to buy an asset, and that this trade will increase its price, to make a profit. The attacker’s plan is to buy this asset cheap, let the victim buy at an increased price, and then sell the received amount again at a higher price afterwards. In both stated contracts, `minAmountA` and `minAmountB` are provided in the `deploy` function. However, they are not checked after addLiquidity is called.

Every Solidity file specifies in the header a version number of the format `pragma solidity (^)0.4.*`. The caret (`^`) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version _and above_, hence the term "unlocked". Different versions of Solidity is used: ``` - Version used: ['0.6.11', '>=0.4.24<0.8.0', '>=0.5.0', '>=0.6.0', '>=0.6.0<0.8.0', '>=0.6.2', '>=0.6.2<0.8.0', '^0.6.0', '^0.6.5'] - ^0.6.5 (node_modules/@0x/contracts-erc20/contracts/src/v06/IERC20TokenV06.sol#20) - ^0.6.5 (node_modules/@0x/contracts-utils/contracts/src/v06/LibSafeMathV06.sol#20) - ^0.6.5 (node_modules/@0x/contracts-utils/contracts/src/v06/errors/LibRichErrorsV06.sol#20) - ^0.6.5 (node_modules/@0x/contracts-utils/contracts/src/v06/errors/LibSafeMathRichErrorsV06.sol#20) - ^0.6.5 (node_modules/@0x/contracts-zero-ex/contracts/src/errors/LibNativeOrdersRichErrors.sol#20) - ^0.6.5 (node_modules/@0x/contracts-zero-ex/contracts/src/errors/LibSignatureRichErrors.sol#20) - ^0.6.5 (node_modules/@0x/contracts-zero-ex/contracts/src/features/interfaces/INativeOrdersEvents.sol#20) - ABIEncoderV2 (node_modules/@0x/contracts-zero-ex/contracts/src/features/interfaces/INativeOrdersEvents.sol#21) - ^0.6.5 (node_modules/@0x/contracts-zero-ex/contracts/src/features/interfaces/INativeOrdersFeature.sol#20) - ABIEncoderV2 (node_modules/@0x/contracts-zero-ex/contracts/src/features/interfaces/INativeOrdersFeature.sol#21) - ^0.6.5 (node_modules/@0x/contracts-zero-ex/contracts/src/features/libs/LibNativeOrder.sol#20) - ABIEncoderV2 (node_modules/@0x/contracts-zero-ex/contracts/src/features/libs/LibNativeOrder.sol#21) - ^0.6.5 (node_modules/@0x/contracts-zero-ex/contracts/src/features/libs/LibSignature.sol#20) - ABIEncoderV2 (node_modules/@0x/contracts-zero-ex/contracts/src/features/libs/LibSignature.sol#21) - >=0.6.0 (node_modules/@chainlink/contracts/src/v0.6/interfaces/AggregatorV3Interface.sol#2) - ^0.6.0 (node_modules/@gnosis.pm/mock-contract/contracts/MockContract.sol#1) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/math/MathUpgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/math/SafeMathUpgradeable.sol#3) - >=0.4.24<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/proxy/Initializable.sol#4) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/SafeERC20Upgradeable.sol#3) - >=0.6.2<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/utils/EnumerableSetUpgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/access/AccessControl.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/access/Ownable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/cryptography/ECDSA.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/cryptography/MerkleProof.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/math/Math.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/math/SafeMath.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/presets/ERC20PresetMinterPauser.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/token/ERC20/ERC20.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/token/ERC20/ERC20Burnable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/token/ERC20/ERC20Pausable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/token/ERC20/IERC20.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/token/ERC20/SafeERC20.sol#3) - >=0.6.2<0.8.0 (node_modules/@openzeppelin/contracts/utils/Address.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/utils/Context.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/utils/EnumerableSet.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/utils/Pausable.sol#3) - >=0.6.0<0.8.0 (node_modules/@openzeppelin/contracts/utils/SafeCast.sol#3) - >=0.5.0 (node_modules/@sushiswap/core/contracts/uniswapv2/interfaces/IUniswapV2ERC20.sol#3) - >=0.5.0 (node_modules/@sushiswap/core/contracts/uniswapv2/interfaces/IUniswapV2Factory.sol#3) - >=0.6.2 (node_modules/@sushiswap/core/contracts/uniswapv2/interfaces/IUniswapV2Router01.sol#3) - >=0.6.2 (node_modules/@sushiswap/core/contracts/uniswapv2/interfaces/IUniswapV2Router02

The `cycleDuration`, which is set in the `initialize` and `setCycleDuration` functions, can be set to an unsafe small value. Since miners can manipulate block numbers within a small range, the miners might be able to trigger the `completeRollover` earlier or later than the intended `cycleDuration`.