Derify protocol security record

"Calling a `nonReentrant` function from another `nonReentrant` function is not supported." See https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.4.2/contracts/security/ReentrancyGuard.sol.

According to the Derify Protocol’s [insurance pool mechanism](https://derify.medium.com/how-does-the-insurance-pool-in-derify-works-c8887ceda8c5), when users withdraw their funds but the system is in a net loss, funds from the insurance pool will be used as a supplement to ensure the normal settlement of the fund withdrawal; when users withdraw their funds but the system is in a net profit, funds from the liquidity pool will be transferred to the insurance pool. See the code snippet provided below: ```solidity function withdraw(uint256 amount) external override nonReentrant whenNotPaused { ... if (compensation > 0) { uint256 profitCompensation = uint256(compensation); if (insuranceFundPool >= profitCompensation) { _transferPoolFrom( PoolUpdateType.ProfitCompensation, AccountType.InsuranceFundPool, AccountType.LiquidityPool, profitCompensation ); } else { // handle the case that InsuranceFundPool not enough to pay => pay bond uint256 pnlBond = profitCompensation - insuranceFundPool; executeAmount = amount - pnlBond; _transferPoolFrom( PoolUpdateType.ProfitCompensation, AccountType.InsuranceFundPool, AccountType.LiquidityPool, insuranceFundPool ); IDerifyRewards(rewards).payBond(trader, pnlBond); } } else if (compensation < 0){ uint256 lossCompensation = uint256(compensation * -1); _transferPoolFrom( PoolUpdateType.LossCompensation, AccountType.LiquidityPool, AccountType.InsuranceFundPool, lossCompensation ); } } ``` However, the smart contract's deposit function possesses a logical inconsistency in the compensation mechanism. When the system is in a net loss, funds from the liquidity pool will be transferred to the insurance pool. This misalignment in fund transfer logic could create discrepancies in compensation, affecting the protocol's reliability.

The function `deposit()` is called by the user to deposit assets. When the `sysNetPnl` of the system is positive and the user attempts to deposit more assets, and the number of assets in the current pool `LiquidityPool` is less than `profitCompensation`, then the function will fail due to the underflow of `uint256`. In other words, the user cannot deposit more assets at that time. File:DerifyExchange.sol ```solidity=80 uint256 public override liquidityPool; ``` ```solidity=149 _transferPoolFrom( PoolUpdateType.ProfitCompensation, AccountType.LiquidityPool, AccountType.InsuranceFundPool, profitCompensation ); ``` ```solidity=161 liquidityPool += amount; ``` Since the amount of `liquidityPool` is accrued on line 161, there may be insufficient assets when transferring the token on line 149 of the contract.

The above-mentioned function has two modifiers `nonRentrant`.

When transferring deflationary ERC20 tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee), only 90 tokens actually arrived to the contract. However, a failure to discount such fees may allow the same user to withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction. Reference: https://thoreum-finance.medium.com/what-exploit-happened-today-for-gocerberus-and-garuda-also-for-lokum-ybear-piggy-caramelswap-3943ee23a39f ```solidity=81 require(IERC20(token).transferFrom(msg.sender, address(this), amount), "CVV_LT: TOKEN_TRANSFER_ERR"); ``` - Transferring tokens by `amount`. ```solidity=79 totalVestingAmount = totalVestingAmount + amount; ``` - The `amount` appears to be used for bookkeeping purposes without compensating the potential transfer fees. --- ```solidity=82 require(IERC20(token).transferFrom(msg.sender, address(this), amount), "PVV_LT: TOKEN_TRANSFER_ERR"); ``` - Transferring tokens by `amount`. ```solidity=80 totalVestingAmount = totalVestingAmount + amount; ``` - The `amount` appears to be used for bookkeeping purposes without compensating the potential transfer fees. --- ```solidity=81 require(IERC20(token).transferFrom(msg.sender, address(this), amount), "TVV_LT: TOKEN_TRANSFER_ERR"); ``` - Transferring tokens by `amount`. ```solidity=79 totalVestingAmount = totalVestingAmount + amount; ``` - The `amount` appears to be used for bookkeeping purposes without compensating the potential transfer fees.

The value of the public constant CONTRACT_DECIMAL is 10e8 in the DerifyBroker contract, hence all the accounts' decimals should be 8. While USD, DRF, and EDRF are with decimals 18. So the protocol has to interact with USD contract and handle the different decimals.

If the account is liquidated, his/her account balance will be cleared.

The amount of the fee is not relevant to the actual `operatedSize`. As long as the position is processed, there will be a flat fee.

If the updates of gasFeePool are always together with the updates of gasBalance, then the two events can be merged for better simplicity.

The function `transferFrom()` in an ERC20 contract is used for receiving the user's payment to the contract. When the contract needs to transfer money out, it's better to use the function `transfer()` because there is the potential that the target token does not support transferring from users selves.