Confti security record

In the below code snippet of `Vote.execute()`, the logic is a little confusing. 1. Both of the require statement and if statement use the condition `_vote.svote.supPopel < _vote.supPopel`, so one of them is redundant. 2. The second require statement uses the opposite condition `_vote.svote.supPopel > _vote.supPopel` and will always roll back the current transaction because the first require statement has already ensures that `_vote.svote.supPopel < _vote.supPopel`. ```solidity=168 require(_vote.svote.supPopel < _vote.supPopel,"The vote passed with fewer votes than the original"); if(_vote.svote.supPopel < _vote.supPopel){ _vote.status = VoteStatus.faild; IVault(getVault()).safeSetState(address(0),0,State.NftState.freedom,"vote"); return; } require(_vote.svote.supPopel > _vote.supPopel,"The vote passed with fewer votes than the original"); _vote.status = VoteStatus.succeed; _execute(voteId_,_vote.codeModel,_vote.value); ``` Based on our understanding, `_vote.svote.supPopel < _vote.supPopel` means that the rejected vote does not pass and the original vote should be executed, while `_vote.svote.supPopel > _vote.supPopel` means that the reject vote passes and the original vote should not be executed. The normal if-else statement can handle this logic well.

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. --- ### External call(s) ```solidity=824 IDivision(getToken()).mintDivision(_sender, amount); ``` ### State variables written after the call(s) ```solidity=825 totalClaimedReward += amount; ``` --- ### External call(s) ```solidity=162 IVault(getVault()).safeSetState(address(0),0,State.NftState.freedom,"vote"); ``` ### State variables written after the call(s) ```solidity=164 executeQueueMap[voteId_] = true; ```

The functions which are never called internally within the contract should have external visibility for gas optimization. ```solidity=259 function getNftState(address nft,uint256 nftId) public override view returns(State.NftState){ ``` --- ```solidity=269 function getNftActivity(address nft,uint256 nftId) public override view returns(string memory){ ``` --- ```solidity=275 function getEntireVaultActivity() public override view returns(string memory){ ``` --- ```solidity=279 function getFreedomNFT() public override view returns(uint256[] memory) { ``` --- ```solidity=287 function getVeToken() public view returns(address) { ``` --- ```solidity=291 function onERC721Received(address, address, uint256, bytes memory) public virtual returns (bytes4) { ``` --- ```solidity=295 function onERC1155Received(address, address, uint256, uint256, bytes memory) public virtual returns (bytes4) { ``` --- ```solidity=299 function onERC1155BatchReceived(address, address, uint256[] memory, uint256[] memory, bytes memory) public virtual returns (bytes4) { ``` --- ```solidity=81 function getUserPointEpoch(address _userAddress) ``` --- ```solidity=99 function getUserPointHistory(address _userAddress, uint256 _index) ``` --- ```solidity=1008 function totalClaimable() public override view returns(uint256){ ``` --- ```solidity=15 function initialize() override public{ ``` --- ```solidity=350 function hashCompareInternal(string memory a, string memory b) public returns (bool) { ``` --- ```solidity=24 function updateRouterUtils() public{ ``` --- ```solidity=28 function updateAuctionUtils() public{ ``` --- ```solidity=33 function updateVaultUtils() public{ ``` --- ```solidity=37 function updateVoteUtils() public { ``` --- ```solidity=42 function updateVeTokenUtils() public{ ``` --- ```solidity=20 function destruct() public { ``` --- ```solidity=17 function pledge()public { ``` --- ```solidity=22 function balanceOf(address add)public view returns(uint256 result){ ``` --- ```solidity=26 function totalSupply()public view returns(uint256 result){ ``` --- ```solidity=30 function setMaxTime(uint256 value)public { ``` --- ```solidity=42 function initialize() public { ```

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=42 settings = settings_; ``` - `settings_` is not zero-checked before being used. --- ```solidity=50 settings = newSettings; ``` - `newSettings` is not zero-checked before being used. --- ```solidity=143 (bool _ok, bytes memory returnData) = _updataTemplate.delegatecall(abi.encodeWithSignature( "updateAuctionUtils()" )); ``` - `_updataTemplate` is not zero-checked before being used. --- ```solidity=43 veTokenTemplate = veTokenTemplate_; ``` - `veTokenTemplate_` is not zero-checked before being used. --- ```solidity=44 vaultTemplate = vaultTemplate_; ``` - `vaultTemplate_` is not zero-checked before being used. --- ```solidity=45 auctionTemplate = auctionTemplate_; ``` - `auctionTemplate_` is not zero-checked before being used. --- ```solidity=46 voteTemplate = voteTemplate_; ``` - `voteTemplate_` is not zero-checked before being used. --- ```solidity=47 divisionTemplate = divisionTemplate_; ``` - `divisionTemplate_` is not zero-checked before being used. --- ```solidity=66 curator = curator_; ``` - `curator_` is not zero-checked before being used. --- ```solidity=212 (bool _ok, bytes memory returnData) = updataTemplate.delegatecall(abi.encodeWithSignature( "updateRouterUtils()" )); ``` - `updataTemplate` is not zero-checked before being used. --- ```solidity=305 (bool _ok, bytes memory returnData) = updataTemplate.delegatecall(abi.encodeWithSignature( "updateVaultUtils()" )); ``` - `updataTemplate` is not zero-checked before being used. --- ```solidity=1129 (bool _ok, bytes memory returnData) = _updataTemplate.delegatecall(abi.encodeWithSignature( "updateVeTokenUtils()" )); ``` - `_updataTemplate` is not zero-checked before being used. --- ```solidity=17 newVoteTemplate = newVoteTemplate_; ``` - `newVoteTemplate_` is not zero-checked before being used. --- ```solidity=18 newRouterTemplate = newRouterTemplate_; ``` - `newRouterTemplate_` is not zero-checked before being used. --- ```solidity=19 newVeTokenTemplate = newVeTokenTemplate_; ``` - `newVeTokenTemplate_` is not zero-checked before being used. --- ```solidity=20 newAuctionTemplate = newAuctionTemplate_; ``` - `newAuctionTemplate_` is not zero-checked before being used. --- ```solidity=21 newVaultTemplate = newVaultTemplate_; ``` - `newVaultTemplate_` is not zero-checked before being used.

The contract `EtherToken` in `wETH.sol` implements the WETH. However, there is no usage of it. The `Vault` contract is still using the official WETH on the chain. ```solidity address public constant WETH = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; ```

The second linked require statements is redundant. If the first statement condition is satisfied, the second statement is also satisfied. ```solidity=138 require(IVault(getVault()).getEntireVaultState() == State.NftState.freedom, "The vault and the auction"); require(IVault(getVault()).getEntireVaultState() != State.NftState.leave, "The vault is closed"); ```

The return value of the transfer()/transferFrom() call is not checked. ```solidity=76 IERC20(token).transfer(to,amount); ``` --- ```solidity=82 IWETH(WETH).transfer(who, IWETH(WETH).balanceOf(address(this))); ``` --- ```solidity=359 IERC20(getToken()).transferFrom( _provider, address(this), _value ); ``` --- ```solidity=552 IERC20(getToken()).transfer(msg.sender, _value); ```

One or more state variables are never used in the codebase. Variable `DEPOSIT_FOR_TYPE` in `VeToken` is never used in `VeToken`. ```solidity=14 uint256 private constant DEPOSIT_FOR_TYPE = 0; ```

It is not recommended to use Solidity's `transfer()` and `send()` functions for transferring Ether, since some contracts may not be able to receive the funds. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=61 payable(getVault()).transfer(_auction.livePrice); ``` - `Auction.start` uses `transfer()`. --- ```solidity=71 who.transfer(amount); ``` - `Vault.sendETH` uses `transfer()`.