Confti security record

In the current protocol, the users can use the `Factory.mint()` function to create `router` contracts to become the `curator` and sell their NFT. With the created `router` contract, the `curator` will create associated contracts like `Auction`, `Vault`, etc. And some Division tokens will be minted to the `curator`. With the minted Division tokens, the `curator` can call the function `Vault.redeem()` to get the unsold NFT back, get the auction profit by `Router.cash()` or locking into the `VeToken` for more Division tokens. Holding Division token will also give the users some voting power to change the configuration variables or even contract implementation of the current contracts. And then, there are some questions: 1. All the deposited NFT tokens are put into the auction and sold at one-time. By calling the below function in the `Auction.end()` function, all the NFT tokens in the `freedomNFTInNFTSIndex` list will be sent to the address `_auction.winning`. The auction only ensures that the total sell price is greater than `auctions[0].price` and does not consider the price of each NFT tokens. ```solidity=104 IVault(getVault()).sendNFTAssetIndex(IVault(getVault()).getFreedomNFT(),_auction.winning); ``` 2. All the Division tokens are sent to the `curator` account, except the 1% `_govAmount` fee and reserved tokens. The `curator` is the one who sells the NFT tokens, so he/she should have some privileges. However, all the voting power are hold by the `curator` and it seems meaningless to have voting and locking logic. 3. The contracts `Auction`, `Router`, `Vault`, `VeToken` and `Vote` can be updated arbitrarily by the one holding most of the Division token. This provide flexibility to the NFT seller `curator`, but increases the potential security risks.

The function `claimFees()` in the contract `Router` uses the state variable `fee` to calculating the annual fee. However, the `fee` variable is defaulted as 0 and set to 0 in the function `issue()`. Also, there is no set functions to change the value of the `fee` variable. Thus, the resulting `currentAnnualFee` and subsequent results `feePerSecond` and `curatorMint` are always 0, which also makes this function redundant. ```solidity=152 uint256 currentAnnualFee = fee * IERC20(division).totalSupply() / 1000; ```

In the contract `Route`, the role `curator` can deposit NFT tokens and sell them. The state variable `open` will be set to true when `issue()` function performing the setup of all the associated contracts. However, after the `Auction.end()` or `Vault.redeem()` which will send all the NFT tokens to the bid winner or the person holding enough `Division` balance, the `open` variable is still true. As a result, the `curator` cannot deposit more NFT tokens and the left `Division` tokens are useless. The `curator` has to create another `Router` with `Factory.mint()`.

In the contract `Auction`, only `auctions[0]` is actually used. The `auctions[_nftInVaultIndex].price` is set but never used.

The function `mintToScale()` has the function modifier `onlyWhiteList` to restrict that only the whitelisted account can call it. However, in the functions `initialize()` and `issue()`, only the associated contracts are included in the white list and these contracts does not have the functionality to call the function `mintToScale()`. The Division token contract also require the caller is `routerContract.whiteList()`. Also the function `setWhiteList()` is private, so other accounts cannot be included into the white list. As a result, the function `mintToScale()` seems to be redundant because no one can call it.

The function call that sends Ether to an arbitrary address is unprotected. ```solidity=69 function sendETH(address payable who,uint256 amount) external override checkSender nonReentrant { require(who != address(0),"sendETH :: Cannot transfer to zero address"); who.transfer(amount); } function sendERC20(address token,address to,uint256 amount) external override checkSender nonReentrant { require(to != address(0),"sendERC20 :: Cannot transfer to zero address"); IERC20(token).transfer(to,amount); } function sendWETH(address who, uint256 amount) external override checkSender nonReentrant{ require(who != address(0),"sendWETH :: Cannot transfer to zero address"); IWETH(WETH).deposit{value: amount}(); IWETH(WETH).transfer(who, IWETH(WETH).balanceOf(address(this))); } ```

In the contract `Router`, the `curator` will deposit NFT tokens for sell and initial associated contracts. In the meantime, the Division tokens are minted to the `curator`. Among the minted Division tokens, 1% is for fee and `reserveRatio / 10000` amount will be left in the `Router` contract. The rest of the minted tokens will be given to the `curator`. The `reserveRatio` is set by the `curator` by passing-in variable. However, we did not see any benefit for the `curator` to set the `reserveRatio` and left Division tokens in the `Router` contract. It will decrease his/her voting power and he/she cannot get the deposited NFT back through the `Vault.redeem()` function since it requires `accountBalance >= _totalAmount - govBalance`(no reserve Division tokens). Thus, we would like to know that what is the `reserveRatio` and reserved Division token using for?

One or more state variables are used without being initialized in the constructor. ```solidity=23 mapping(uint256=> GgovernanceTemplate) template; ``` - `template` is never initialized, but used in `Vote.getTemplate`.

In the contract `Settings`, the state variable `feeReceiver` is initialized as `0xAEb639aa794055Bc1992fB06545fdF2cFF53DC10` and can be changed after deploying. The ownership of it is uncertain.

The calculation of the slope for a locked amount is the amount divided by `MAXTIME = 60,480,000`. ```solidity=169 if (_oldLocked.end > block.timestamp && _oldLocked.amount > 0) { _st.userOldPoint.slope = _oldLocked.amount / int256(maxPledgeDuration); _st.userOldPoint.bias = _st.userOldPoint.slope * int256(_oldLocked.end - block.timestamp); } if (_newLocked.end > block.timestamp && _newLocked.amount > 0) { _st.userNewPoint.slope = _newLocked.amount / int256(maxPledgeDuration); _st.userNewPoint.bias = _st.userNewPoint.slope * int256(_newLocked.end - block.timestamp); } ``` A consequence of this is that locked amounts less than `MAXTIME` will have zero voting power.