DerivaDEX security record

The `initialize()` function member of `DDXWalletCloneable` is used to set the wallets that hold the traders DDX fund. When creating the clone contract in Trader the constructor is not called, however, the `initialize()` function can be called later on to set the approval and delegation. Once the clone created and initialized, any attacker can re-initialize the clone since initialize does not require to be intialized only once. As a result all DDX funds can be taken since the attacker can approve the tokens to his own address.

The function `getPriorVotes()` load an entire Checkpoints structure `cp = checkpoints[_account]` to memory from storage. The checkpoint structure contains two dynamic arrays, thus, an attacker can start an attack where he delegates his voting power to an important voting address with high weight, causing the delegatee Checkpoint arrays toincrease in size when the attacker make transfers back and forth between his addresses. If the array sizes increase enough the attacker can render voting inacessible for the delegatee address, since `getPriorVotes()` will consume excessive gas when loading the arrays into the memory and when the while loop will execute, and therefore increase his chances to pass a proposal. `getPriorVotes()` checks the most recent balance before loading the storage to the memory and executing the fetching algorithm, however, this can be bypassed by making a token transfer from the attacker address that delegates its voting power to the attacked address when the proposal voting starts. A more sophisticated attack can involve a malicous contract that own sub-contracts that attack multiple delegees addresses in a single transaction; please note that each token transfer transaction has to be mined in a different block to increase the arrays length.

The function does not validate either of the `s` and `v`. values. See [ECDSA.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/cryptography/ECDSA.sol#L46).

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. In particular: * If the number of checkpoints is very large, `advanceInsuranceMineInterval()` may run into gas issues. * In `claimDDXFromInsuranceMining()`, if the staker has not claimed rewards in many intervals, there may be gas issues.

Several functions do not fully sanitize for faulty arguments. Constructors and initializer functions should ensure that all relevant address arguments are non-zero to avoid faulty deployments. Setter functions should ensure that the underlying state of the contract remains consistent, e.g., variables that constitute lower bounds should be less than variables that denote upper bounds. In the function `DDX.transferOwnershipToDerivaDEXProxy()` is allowed to be called only once and can be subject to input error. We recommend ensuring that `_derivaDEXProxy != address(0)`. When setting `DDX` address in the `DerivaDEX` constructor, `DDX` is not checked to be different than `address(0)`. In `Governance.sol`: * `skipRemainingVotingThreshold` has to be higher than `quorumVotes` otherwise all proposal can eventually reach a defeated state following the `state()` function implementation. If the contract is self governed and a change is made where the `skipRemainingVotingThreshold` is lower than the `quorumVotes`, there will be a single way for reverting this situation since all future proposals will fail except if the last vote is high enough to reach both `skipRemainingVotingThreshold` and `quorumVotes` at the same time when the vote is casted. * When setting governance parameters, some rules should be enforced programmatically such as the `minimumDelay` and `maximumDelay`. As an example, if `maximumDelay` is set lower than `minimumDelay`, all proposal will be rejected. * The function `castVote()` does not revert if the `msg.sender` address votes are equal to zero. In `InsuranceFund.sol`: * The function `setAdvanceIntervalReward()` sets the weight drop between intervals. This should likely be bounded above by 100 through a require-statement. * Similarly for `setWithdrawalFactor()`, this should be capped to 1000. * The address parameters to `addInsuranceFundCollateral()` should at least be ensured to be non-zero. In `Trader.sol`, `setRewardCliff()` may require a bound (possibly 100).

When staking, `stakeToInsuranceFund()` will issue an amount of `DIFundToken` for the `msg.sender` equal to `_amount.proportion96(multiplier, 10)` (please note that the minted amount of `DIFundToken` is used to track the user deposited stake) where the multiplier is used as incentive. Users with higher multiplier value will be issued a higher value of `DIFundToken` and the opposite is true. When withdrawing, `withdrawFromInsuranceFund()` will burn the `DIFundToken` input `_amount` and calculate the collateral amount to give back to the `msg.sender` (`underlyingToTransferNoFee`). If different multiplier values are used when the users stake, the users that get a higher multiplier can get a higher collateral amount withdrawal than his initial deposit (the opposite is also true for lower multiplier value), since they get the proportion of `claimCheckpoint.cap` divided by `stakeCollateral.diFundToken.getTotalPriorValues(block.number.sub(1))` of the input `DIFundToken` to be burned. Please note that a users that stake `X` amount of collateral, in a normal scenario, it is expected that they get the same value of collateral returned when burning the totality of his issued `DIFundToken`.

Setting `skipRemainingVotingThreshold` value lower than 51% of the `DDX` supply or the total voting power will incentivize the voters to race against each other since the first group to reach that value will most probably win by either defeating the proposal of succeeding, since the voting deadline won't be necessary as seen [here](https://github.com/apalepu23/derivadex_v1/blob/6bb9ac918e87bc416966b385cc5800cd7c310953/packages/protocol/contracts/facets/governance/Governance.sol#L754). Following the importance of the proposal or if the proposal contains malicous code or actions, using `skipRemainingVotingThreshold` can help attackers reach their goal faster.

Total Supply is the total amount of coins in existence at any moment. The DDX token `totalSupply()` function returns the `TOTAL_SUPPLY` state variable (please note that `TOTAL_SUPPLY` is the max cap and not the existing supply and the state variable is set without being fully minted); `circulatingSupply` is used instead to count the existing token supply that is minted progressively. When burning tokens, the amount is deducted only from `circulatingSupply` and not from `TOTAL_SUPPLY` making the contract return wrong estimation of the existing tokens when calling `totalSupply()`.

The `DerivaDEX` contract implement receive method to handle transaction with empty `msg.data` to receive ether instead of the fallback function. However, no function is implemented to handle ether in the contract balance; any funds sent by mistake will be frozen except if a facet with a `withdraw()` function is added.

Even if `getCurrentVotes()` is not used by the implemented Governance facet, using it in future iteration may make the system vulnerable to a flashloans vote, since it includes the user last CheckPoints even if it was created in the same block.