In the contract `CRPFactory` the role `_blabs` has authority over the functions shown in the diagram below.
Any compromise to the `_blabs` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_CRPFactory-CRPFactory-_blabs.svg)

Centralization Risks in CRPFactory.sol

In the contract `ConfigurableRightsPool` the role `_owner` has authority over the functions shown in the diagram below.
Any compromise to the `_owner` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_ConfigurableRightsPool-ConfigurableRightsPool-_owner.svg)

In the contract `ConfigurableRightsPool` the role `admin` has authority over the functions shown in the diagram below.
Any compromise to the `admin` account may allow the hacker to take advantage of this authority..
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_ConfigurableRightsPool-ConfigurableRightsPool-admin.svg)

In the contract `ConfigurableRightsPool` the role `module` has authority over the functions shown below.
Any compromise to the `module` account may allow the hacker to take advantage of this authority.
- function execute()

Centralization Risks in ConfigurableRightsPool.sol

The issue fee is not taken into consideration when calculating how many tokens can purchase the desired pool shares. 

The problem this causes is that the actual purchase price is lower than expected. And the further back you go, the less tokens are required to purchase the same number of pool shares. 

### Proof of Concept
Here is an example:
1. Suppose a closed-end pool contains 1000 USDT at first, and the init supply is 100 as well. It means each pool share is worth `$10` without issue fee.
2. Bob purchases 100 pool shares at cost of 1000 USDT. Suppose there is 10% issue fee, the actual amount deposited in pool is 900 USDT. Now the pool has 1900 USDT, and the total supply is 200. It means each pool share is only worth `$9.5` now.
3. This time Alice is able to purchase 100 pool shares at cost of 950 USDT. But with 10% issue fee, the actual amount deposited into the pool is 855 USDT. Now the pool has 2755 USDT, and the total supply is 300. It means each pool share is only worth nearly `$9.18` now.
4. With more and more users taking part in this pool, the tokens required to purchase 100 pool shares gets lower and lower, which seems unreasonable.

In summary, such a design hurts early adopters by driving prices lower and lower. This will discourage users from buying, which is clearly detrimental to the project.

Mint Pool Share Without Consideration of Issue Fee

The state variable `result` and `isFirstBigger` is shadowing the returned variables. In this case, the return value is always `(0,false)`. This in turn makes it likely that the price obtained by `getPrice()` is wrong.

```solidity=71
    function abs(uint a, uint b) internal pure returns(uint result, bool isFirstBigger) {
        uint result;
        bool isFirstBigger;
        if(a > b){
            result = a - b;
            isFirstBigger = true;
        } else {
            result = b - a;
            isFirstBigger = false;
        }
    }
```

```solidity=99
    function getPrice(address tokenAddress) public returns (uint price) {
        IERC20 token = IERC20(tokenAddress);
        AggregatorV2V3Interface feed = getFeed(token.symbol());
        if (prices[address(token)] != 0) {
            price = prices[address(token)];
        } else if (address(feed) != address(0)) {
            price = getChainlinkPrice(feed);
        } else {
            try twapOracle.update(address(token)) {} catch {}
            price = getUniswapPrice(tokenAddress);
        }

        (uint decimalDelta, bool isUnderFlow18) = uint(18).abs(uint(token.decimals()));
        ...
```

Shadowing State Variable

In the contract `DesynChainlinkOracle` the role `admin` has authority over the functions shown in the diagram below.

Any compromise to the `admin` account may allow the hacker to take advantage of this authority to directly manipulate prices.

![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_DesynChainlinkOracle-DesynChainlinkOracle-admin.svg)

Centralization Risks in DesynChainlinkOracle.sol

In the contract `DesynOwnable` the role `_owner` has authority over the functions shown in the diagram below.
Any compromise to the `_owner` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_DesynOwnable-DesynOwnable-_owner.svg)

Centralization Risks in DesynOwnable.sol

In the contract `Factory` the role `_blabs` has authority over the functions shown in the diagram below.
Any compromise to the `_blabs` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_Factory-Factory-_blabs.svg)

Centralization Risks in Factory.sol

In the contract `LiquidityPool` the role `_controller` has authority over the functions shown in the diagram below.
Any compromise to the `_controller` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_LiquidityPool-LiquidityPool-_controller.svg)

Centralization Risks in LiquidityPool.sol

In the contract `Oracle` the role `dev_address` has authority over the functions shown in the diagram below.
Any compromise to the `dev_address` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_Oracle-Oracle-dev_address.svg)

Centralization Risks in Oracle.sol

File: SmartPoolManager.sol
```solidity=219
    function exitPoolHandle(
        uint _endEtfAmount,
        uint _endFundAmount,
        uint _beginEtfAmount,
        uint _beginFundAmount,
        uint poolAmountIn,
        uint totalEnd
    )
        external
        pure
        returns (
            uint endEtfAmount,
            uint endFundAmount,
            uint profitRate
        )
    {
        endEtfAmount = DesynSafeMath.badd(_endEtfAmount, poolAmountIn);
        endFundAmount = DesynSafeMath.badd(_endFundAmount, totalEnd);
        uint amount1 = DesynSafeMath.bdiv(endFundAmount, endEtfAmount);
        uint amount2 = DesynSafeMath.bdiv(_beginFundAmount, _beginEtfAmount);
        if (amount1 > amount2) {
            profitRate = DesynSafeMath.bdiv(
                DesynSafeMath.bmul(DesynSafeMath.bsub(DesynSafeMath.bdiv(endFundAmount, endEtfAmount), DesynSafeMath.bdiv(_beginFundAmount, _beginEtfAmount)), poolAmountIn),
                totalEnd
            );
        }
    }
```

Since `beginFund[manager].etfAmount` and `beginFund[manager].fundAmount` are not initialized, the manager can not withdraw his/her funds unless the manager joins the pool again during the fundraising.

File: ConfigurableRightsPool.sol
```solidity=365
    if (totalBegin > 0) {
            SmartPoolManager.Fund storage fund = beginFund[msg.sender];
            fund.etfAmount = DesynSafeMath.badd(beginFund[msg.sender].etfAmount, poolAmountOut);
            fund.fundAmount = DesynSafeMath.badd(beginFund[msg.sender].fundAmount, totalBegin);
        }
```

### Proof of Concept
  1. `Alice` creates `CRP` pool and deposits initial funds to the pool.
  2. After a while, `Alice` tries to redeem her funds. She can not reclaim funds since her `_beginEtfAmount` is `0` which causes the transaction to revert.

Initial Supply Locked

In the contract `UserVault` the role `_owner` has authority over the functions shown in the diagram below.
Any compromise to the `_owner` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_UserVault-UserVault-_owner.svg)

In the contract `UserVault` the role `vaultAddress` has authority over the functions shown in the diagram below.
Any compromise to the `vaultAddress` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_UserVault-UserVault-vaultAddress.svg)

In the contract `UserVault` the role `CRP Pool` has authority over the functions shown below.
Any compromise to the `CRP pool` account may allow the hacker to take advantage of this authority.
- function recordTokenInfo()

In the contract `UserVault` the role `CRP Factory` has authority over the functions shown below.
Any compromise to the `CRP Factory` account may allow the hacker to take advantage of this authority.
- function setPoolParams()

Centralization Risks in UserVault.sol

The `kols_list[]` is used to record all the `KOLs`(Key opinion leader) of a pool. However, the `kol` can be pushed into `kols_list` repeatedly without limitation.

```solidity=500
    address newKol = user_kol_list[msg.sender][user];
    if (user_kol_list[msg.sender][user] == address(0)) {
        user_kol_list[msg.sender][user] = kol;
        kols_list[msg.sender].push(kol);
        newKol = kol;
    }
```

As a result, when using the `kols_list` to calculate the reward amount by the function `getAllKolReward()`, the calculated result is much larger than expected. More rewards will be delivered to the `KOLs` and managers of the pool. This will result in a loss of property for the platform, and can even leave rewards unclaimed if the calculated rewards even exceed the actual holding of the platform. 

```solidity=345
    function getAllKolReward(address pool) public view returns (uint totalAmount) {
        address[] memory list = kols_list[pool];
        uint len = list.length;
        for (uint i = 0; i < len; i++) {
            totalAmount += kolClaimTotal(pool, list[i]);
        }
    }
```

### Proof of Concept
Here is an example:
1. Suppose there is only one `KOL` of a closed-end pool, and all the users are attracted by him/her.
2. When `Bob` joins into this pool, the `KOL` will be put into `kols_list`.
3. When `Alice` joins into this pool, this `KOL` will be put into `kols_list` again.
4. With more and more users joining into this pool, this `KOL` will be put into `kols_list` continuously. 
5. Suppose there are eventually 100 participants. If the origin fee rate for the `KOLs` is 1%, it will now be magnified by 100 times, which is 100%.
6. If there are eventually 1000 participants, the 1% fee will even be magnified to 1000%, and the vault may not have enough funds, which will make the claim transaction fail to execute.

KOL May Be Repeatedly Added

The contract `UserVault` is used to collect the fees of `KOLs` and pool managers in closed-end ETF pools. The fees are charged in different kinds of tokens, but the token lists recorded in `UserVault` are only set once and can never be updated.

```solidity
    // UserVault.sol
    //managerFee list
    mapping(address => address[]) public pool_manager_tokenList;
    mapping(address => uint[]) public pool_manager_tokenAmount;
    //issueFee list
    mapping(address => address[]) public pool_issue_tokenList;
    mapping(address => uint[]) public pool_issue_tokenAmount;
    //redeemFee list
    mapping(address => address[]) public pool_redeem_tokenList;
    mapping(address => uint[]) public pool_redeem_tokenAmount;
    //perfermanceFee list
    mapping(address => address[]) public pool_perfermance_tokenList;
    mapping(address => uint[]) public pool_perfermance_tokenAmount;
```

The contract `Vault` has a function `clearPool()` to clear all token lists after the fees being claimed. As a result, there will be no problem when the manager rebalances the pool and changes the token list.

```solidity
    // Vault.sol

    function depositManagerToken(address[] calldata poolTokens, uint[] calldata tokensAmount) public {
        require(crpFactory.isCrp(msg.sender), "invalid address");
        ...
        pool_manager_isClaim[msg.sender] = true;
    }

    function managerClaim(address pool) public {
        ...
        pool_manager_isClaim[pool] = false;
        ...
        clearPool(pool);
    }

    function clearPool(address pool) internal {
        pool_manager_tokenAmount[pool] = new uint[](0);
        pool_manager_tokenList[pool] = new address[](0);
        pool_issue_tokenAmount[pool] = new uint[](0);
        pool_issue_tokenList[pool] = new address[](0);
        pool_redeem_tokenAmount[pool] = new uint[](0);
        pool_redeem_tokenList[pool] = new address[](0);
        pool_perfermance_tokenAmount[pool] = new uint[](0);
        pool_perfermance_tokenList[pool] = new address[](0);
    }
```

However, unlike the contract `Vault`, there is no way to clear/reset the previous token list in the contract `UserVault`. Also, the `pool_manager_isClaim[pool]` is not set to `false` after claim `Kol` reward and claim managers reward.

### Proof of Concept

Suppose such a test case:
1. The manager of a closed-end fund pool inits the pool and the token list is (USDT)
2. Then the manager rebalances the pool, and now the token list is (USDT, BTC)
3. The token list has been changed, but the old list is still used when depositing fees into `UserVault` by the function `depositToken()`, which will cause reverting. Details can be found in the comment code shown below:
```solidity
    // UserVault.sol
    function communaldepositToken(
        address[] calldata poolTokens, // new: (USDT, BTC) 
        uint[] calldata tokensAmount,
        address poolAdr,
        address[] memory _pool_tokenList, // old: (USDT)
        uint[] memory _pool_tokenAmount
    ) internal returns (address[] memory new_pool_tokenList, uint[] memory new_pool_tokenAmount) {
        new_pool_tokenList = new address[](poolTokens.length);
        new_pool_tokenAmount = new uint[](poolTokens.length);
        if ((_pool_tokenList.length == _pool_tokenAmount.length && _pool_tokenList.length == 0) || !pool_manager_isClaim[poolAdr]) {
            for (uint i = 0; i < poolTokens.length; i++) {
                new_pool_tokenList[i] = poolTokens[i];
                new_pool_tokenAmount[i] = tokensAmount[i];
            }
        } else {
            for (uint k = 0; k < poolTokens.length; k++) { // Note: poolTokens.length == 2 
                if (_pool_tokenList[k] == poolTokens[k]) { // _pool_tokenList.length == 1
                    uint tokenBalance = tokensAmount[k];
                    new_pool_tokenList[k] = poolTokens[k];
                    new_pool_tokenAmount[k] = _pool_tokenAmount[k].add(tokenBalance);
                }
            }
        }
        return (new_pool_tokenList, new_pool_tokenAmount);
    }
```

Unchanged Token List in `UserVault`

In the contract `Vault` the role `_owner` has authority over the functions shown in the diagram below.
Any compromise to the `_owner` account may allow the hacker to take advantage of this authority.
![](https://accelerator-tasks-prod.s3.amazonaws.com/0caf53951ac74bd79d5bcad010deb87a/diagrams/centralization_Vault-Vault-_owner.svg)

In the contract `Vault` the role `CRP Pool` has authority over the functions shown below.
Any compromise to the `CRP Pool` account may allow the hacker to take advantage of this authority.
- function depositManagerToken()
- function depositIssueRedeemPToken()

Centralization Risks in Vault.sol

If the controller of `CRP` is changed, the following `require` check can not be passed and hence that the fees can not be claimed anymore. 
```solidity=479
    require(pool_manager[pool] == manager_address,"claim is not manager");
```

This is because the `pool_manager[pool]` only can be set once when the first fee is deposited.
```solidity=242
    if (pool_manager[msg.sender] == address(0)) {
        address manager_address = ICRPPool(msg.sender).getController();
        pool_manager[msg.sender] = manager_address;
    }
```

### Proof of Concept
  1. `Alice` creates the `CRP` pool and becomes the manager.
  2. `Alice` sets `Bob` as the new manager of `CRP`. However, the `manager` recorded in `vault.sol` is still `Alice`, and there is no way to update the manager.
  3. `Bob` can not call the function `managerClaim()` successfully to claim the fee since he is not the manager recorded in `vault.sol`.

Potential Loss of Fees

The `_updateManageHasClaimed()` is not called when it is open ETF but the manager is not a contract. If you do not update the amount claimed, more rewards will be counted when calculating the manager's remaining rewards. This means that the manager can collect a constant stream of rewards until the pool is depleted of the corresponding tokens.

```solidity=391
        if(isOpenETF && isContractManager){
            address[] memory managerAddressList = IDesynOwnable(pool).getOwners();
            uint[] memory ownerPercentage = IDesynOwnable(pool).getOwnerPercentage();
            uint allOwnerPercentage = IDesynOwnable(pool).allOwnerPercentage();

            for (uint k; k < managerAddressList.length; k++) {
                address reciver = address(managerAddressList[k]).isContract()? IDSProxy(managerAddressList[k]).owner(): managerAddressList[k];
                uint b = balance.mul(ownerPercentage[k]).div(allOwnerPercentage);
                IERC20(t).safeTransfer(reciver, b);
                emit ManagerClaim(msg.sender, pool, reciver,t,b,block.timestamp);
            }
            _updateManageHasClaimed(pool,t,balance);
            emit ManagersClaim(msg.sender, pool, t, balance, block.timestamp);
        }

        if(isOpenETF && !isContractManager){
            IERC20(t).safeTransfer(managerAddr, balance);
        }
```

Claimed Amount Not Updated

The contract `Actions` has no access control. This contract should only be used in the conjunction with `ds-proxy`. Calling this contract directly will lead to loss of funds. 

If users call this contract directly, the controller of all the pools(LP or CRP) created by contract `Actions` are contract `Actions` itself. This can be a big problem since anyone can call functions of contract `Actions` to manage the pools which are not belong to him/her.

Lack Of Access Control

If multiple tokens have the same symbol, then conflicts can occur when setting feeds. If the feeds settings are wrong, errors can also occur in subsequent price queries, which can lead to serious consequences.

Symbol Conflict

According to the fee description document provided by the team, the platform will get 20% of all fees by default, and the rest 80% of fees belong to the managers.

```solidity=229
    uint public total_ratio = 1000;
    uint public manager_ratio = 200;
    uint public issue_ratio = 200;
    uint public redeem_ratio = 200;
    uint public perfermance_ratio = 200;
```

```solidity=578
        if (_pool_manager_tokenAmount.length == 0 ? false : true) {
            balanceOne = _pool_manager_tokenAmount[i].mul(manager_ratio).div(total_ratio);
            balance = balance.add(balanceOne);
        }
        if (_pool_issue_tokenAmount.length == 0 ? false : true) {
            balanceTwo = _pool_issue_tokenAmount[i].mul(issue_ratio).div(total_ratio);
            balance = balance.add(balanceTwo);
        }
        if (_pool_redeem_tokenAmount.length == 0 ? false : true) {
            balanceThree = _pool_redeem_tokenAmount[i].mul(redeem_ratio).div(total_ratio);
            balance = balance.add(balanceThree);
        }
        if (_pool_perfermance_tokenAmount.length == 0 ? false : true) {
            balanceFour = _pool_perfermance_tokenAmount[i].mul(perfermance_ratio).div(total_ratio);
            balance = balance.add(balanceFour);
        }
```

The `balance` is the total fee that will be sent to the `UserVault` or `manager_address`, the ratio (like `manager_ratio`) is intended to calculate a fee for the managers. However, the usage here is exactly the opposite of the design.

Wrong Calculation Of Fees

The function `joinPool()` in the contract `ConfigurableRightsPool` has added a new input parameter `kol` in this version, so the interface of the function `joinPool()` in the contract `LiquidityPool` and `ConfigurableRightsPool` are no longer the same.

The function `joinPool()` in the contract `Actions` will fail to execute.

File: Actions.sol
```solidity=217
    function joinPool(
        LiquidityPoolActions pool,
        uint poolAmountOut,
        uint[] calldata maxAmountsIn
    ) external {
        address[] memory tokens = pool.getFinalTokens();
        _join(pool, tokens, poolAmountOut, maxAmountsIn, msg.sender);
    }
```
```solidity=327
    function _join(
        AbstractPool pool,
        address[] memory tokens,
        uint poolAmountOut,
        uint[] memory maxAmountsIn,
        address kol
    ) internal {
        require(maxAmountsIn.length == tokens.length, "ERR_LENGTH_MISMATCH");

        for (uint i = 0; i < tokens.length; i++) {
            ERC20 token = ERC20(tokens[i]);
            require(token.transferFrom(msg.sender, address(this), maxAmountsIn[i]), "ERR_TRANSFER_FAILED");
            _safeApprove(token, address(pool), maxAmountsIn[i]);
        }
        pool.joinPool(poolAmountOut, maxAmountsIn, kol);
        for (uint i = 0; i < tokens.length; i++) {
            ERC20 token = ERC20(tokens[i]);
            if (token.balanceOf(address(this)) > 0) {
                require(token.transfer(msg.sender, token.balanceOf(address(this))), "ERR_TRANSFER_FAILED");
            }
        }
        require(pool.transfer(msg.sender, pool.balanceOf(address(this))), "ERR_TRANSFER_FAILED");
    }
```
File: LiquidityPool.sol
```solidity=377
    function joinPool(uint poolAmountOut, uint[] calldata maxAmountsIn) external _logs_ _lock_ {
        require(_finalized, "ERR_NOT_FINALIZED");

        uint poolTotal = totalSupply();
        uint ratio = bdiv(poolAmountOut, poolTotal);
        require(ratio != 0, "ERR_MATH_APPROX");

        for (uint i = 0; i < _tokens.length; i++) {
            address t = _tokens[i];
            uint bal = _records[t].balance;
            uint tokenAmountIn = bmul(ratio, bal);
            require(tokenAmountIn != 0, "ERR_MATH_APPROX");
            require(tokenAmountIn <= maxAmountsIn[i], "ERR_LIMIT_IN");
            _records[t].balance = badd(_records[t].balance, tokenAmountIn);
            emit LOG_JOIN(msg.sender, t, tokenAmountIn);
            _pullUnderlying(t, msg.sender, tokenAmountIn);
        }
        _mintPoolShare(poolAmountOut);
        _pushPoolShare(msg.sender, poolAmountOut);
    }
```

Incompatibility With `LiquidityPool`

```solidity=123
            return uint(feed.latestAnswer()).mul(10**decimalDelta);
```
- The type conversion `uint256(feed.latestAnswer())` from type int256 to type uint256 may flip the value's sign.


---
```solidity=125
            return uint(feed.latestAnswer());
```
- The type conversion `uint256(feed.latestAnswer())` from type int256 to type uint256 may flip the value's sign.

Unsafe Integer Cast

The return value of the transfer()/transferFrom() call is not checked.

```solidity=290
            IERC20(kol_token_list[pool][0]).transfer(IDSProxy(msg.sender).owner(), totalAmount);
```


---
```solidity=292
            IERC20(kol_token_list[pool][0]).transfer(msg.sender, totalAmount);
```


---
```solidity=307
            IERC20(kol_token_list[pool][0]).transfer(IDSProxy(msg.sender).owner(), totalAmount);
```


---
```solidity=309
            IERC20(kol_token_list[pool][0]).transfer(msg.sender, totalAmount);
```


---
```solidity=611
            IERC20(token).transfer(user, amount);
```


---
```solidity=309
            IERC20(t).transferFrom(msg.sender, address(this), tokenBalance);
```


---
```solidity=318
            IERC20(t).transferFrom(msg.sender, address(this), tokenBalance);
```


---
```solidity=351
            IERC20(poolTokens[i]).transferFrom(msg.sender, address(this), tokenBalance);
```


---
```solidity=359
            IERC20(poolTokens[k]).transferFrom(msg.sender, address(this), tokenBalance);
```


---
```solidity=439
            IERC20(token).transfer(user, amount);
```


---
```solidity=551
            IERC20(t).transfer(managerAddressList[k], balance.mul(ownerPercentage[k]).div(allOwnerPercentage));
```


---
```solidity=554
            IERC20(t).transfer(manager_address, balance);
```


---
```solidity=557
            IERC20(t).transfer(userVault, balance);
```



Unchecked ERC-20 `transfer()`/`transferFrom()` Call

External price oracles may be risky. If price oracles on which the `Desyn` project relies is compromised, the impact could be incalculable. There are two important concerns about oracles.

1. Price acquired from oracles might be outdated.
2. Price could be manipulated. \
   The price could be manipulated by many reasons. For example, an attacker could borrow flash loan to temporarily manipulate the price.

External Oracle Dependencies Risk

In the function `initHandle()`, the input `_owners[i]` can be repeatedly added into the admin list.
```solidity=54
    function initHandle(address[] memory _owners, uint[] memory _ownerPercentage) external onlyOwner {
        require(_owners.length == _ownerPercentage.length, "ownerP");
        for (uint i = 0; i < _owners.length; i++) {
            allOwnerPercentage += _ownerPercentage[i];
            adminList[_owners[i]] = true;
        }
        owners = _owners;
        ownerPercentage = _ownerPercentage;
    }
```

In the function `removeOwner()`, if the input `owner` is not included in the admin list, the last admin will be removed from list mistakenly.
```solidity=96
    function removeOwner(address owner) external onlyOwner {
        adminList[owner] = false;
        uint amount = 0;
        for (uint i = 0; i < owners.length; i++) {
            if (owners[i] == owner) {
                owners[i] = owners[owners.length - 1];
                amount = ownerPercentage[i];
                ownerPercentage[i] = ownerPercentage[ownerPercentage.length - 1];
                break;
            }
        }
        owners.pop();
        ownerPercentage.pop();
        allOwnerPercentage -= amount;
        emit RemoveAdmin(owner, amount);
    }
```

Missing Validation Before Adding/Removing Admins

The contract is serving as the underlying entity to interact with third party protocols(ERC20 tokens, SwapRouter, etc). The scope of the audit treats third party entities as black boxes and assume their functional correctness. However, in the real world, third parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of third parties can possibly create severe impacts, such as increasing fees of third parties, migrating to new LP pools, etc.

Third Party Dependency

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if 100 deflationary tokens(with a 10% transaction fee) are transferred from users to `LP` or `CRP`, only 90 tokens actually arrived in the contract. However, the contract still records 100 as balance, which causes the contract to lose 10 tokens in such a transaction.

Incompatibility with Deflationary Tokens

It is expected that calls of the form `transfer()`/`transferFrom()` fail if the recipient address is the zero address.

Function `transfer` of Contract `LpToken` Does Not Prevent Transfers to the Zero Address

The return value of an external call is not stored in a local or state variable.

```solidity=279
    swapRouter.swapExactTokensForTokens(deltaBalance, minAmountOut, paths, address(this), badd(block.timestamp, 1800));
```


---
```solidity=308
    swapRouter.swapExactTokensForTokens(deltaBalance, minAmountOut, paths, address(this), badd(block.timestamp, 1800));
```


---
```solidity=428
    token.approve(spender, 0);
```


---
```solidity=430
    token.approve(spender, amount);
```


---
```solidity=38
    token.approve(spender, 0);
```



Unused Return Value

Addresses should be checked before assignment or external call to make sure they are not zero addresses.

```solidity=156
        _controller = manager;
```
- `manager` is not zero-checked before being used.


---
```solidity=27
        owner = owner_;
```
- `owner_` is not zero-checked before being used.


---
```solidity=160
        admin = newAdmin;
```
- `newAdmin` is not zero-checked before being used.


---
```solidity=104
        _blabs = b;
```
- `b` is not zero-checked before being used.


---
```solidity=121
        _swapRouter = router;
```
- `router` is not zero-checked before being used.


---
```solidity=151
        _oracle = oracle;
```
- `oracle` is not zero-checked before being used.


---
```solidity=165
        _vaultAddress = newVault;
```
- `newVault` is not zero-checked before being used.


---
```solidity=174
        _userVaultAddress = newVault;
```
- `newVault` is not zero-checked before being used.


---
```solidity=183
        _managerOwner = newManagerOwner;
```
- `newManagerOwner` is not zero-checked before being used.



Missing Zero Address Validation

Simply put, if there is a profit, a performance fee is charged. But we would like to confirm the exact rules for charging `performanceFee`. There are two main cases when users redeem pool shares.

**Case 1: Redeem all at once** 
1. `Bob` purchased 200 pool shares for a total price of `$1000`. The initial price was `$5` per share. The `performanceFee` rate is 1%.
2. Now with the rise of the token price, the share price came to `$8` per share. `Bob` sold all his 200 shares for a total of `$1600`.

In this case, the `performanceFee` was `$6` according to the current formula.

We have no doubts about this case.

**Case 2: Redeem all at many times**
1. `Bob` purchased 200 pool shares for a total price of `$1000`. The initial price was `$5` per share. The `performanceFee` rate is 1%.
2. Now with the rise of the token price, the share price came to `$8` per share. But `Bob` only sold his 100 shares for a total of `$800`. This time, the `performanceFee` was `$3` according to the current formula.
3. After a period of time, the share price came to `$4` per share. And `Bob` sold his last 100 shares for a total of `$400`. This time, the `performanceFee` was `$1` according to the current formula.

In this case, the total `performanceFee` was `$4` according to the current formula. 

We would like to know what is the exact rules for charging `performanceFee` when an user redeems pool shares at many times? Especially the second redemption in this case, since it makes no profit, should it be fee free in this situation?

Rules for Charging `performanceFee`

If a closed-end pool has not collected enough funds, the issue/redeem/performance fees do not need to be charged according to current code logic. But the manager fee can still be charged since there is no restriction in the function `claimManagerFee()` of the contract `ConfigureRightsPool`. 

File: ConfigureRightsPool.sol
```solidity=206
    function claimManagerFee() external virtual logs lock onlyManager needsBPool {
        uint lastClaimTime = DesynSafeMath.bsub(block.timestamp, etfStatus.startClaimFeeTime);
        require(lastClaimTime >= claimPeriod, "The collection cycle is not reached");
        uint time = lastClaimTime / claimPeriod;
        address[] memory poolTokens = bPool.getCurrentTokens();
        uint[] memory tokensAmount = new uint[](poolTokens.length);
        for (uint i = 0; i < poolTokens.length; i++) {
            address t = poolTokens[i];
            uint tokenBalance = bPool.getBalance(t);
            uint tokenAmountOut = DesynSafeMath.bmul(tokenBalance, (etfStatus.managerFee * time) / 12);
            _pushUnderlying(t, address(this), tokenAmountOut);
            IERC20(t).safeApprove(vaultAddress, tokenAmountOut);
            tokensAmount[i] = tokenAmountOut;
        }
        IVault(vaultAddress).depositManagerToken(poolTokens, tokensAmount);
        etfStatus.startClaimFeeTime = etfStatus.startClaimFeeTime + time * claimPeriod;
    }
```

However, the manager fees can not be claimed by `KOLs`/managers according to the following code logic. The manager fees will be locked in the contract `UserVault`. Only the owner of this contract can withdraw these manager fees by calling the function `adminClaimToken()`.

File: UserVault.sol
```solidity=285
   require(ICRPPool(pool).isCompletedCollect(), "Not CompletedCollect");
```
```solidity=301
   require(ICRPPool(pool).isCompletedCollect(), "Not CompletedCollect");
```

Rules for Charging `managerFee`

There is no validation to ensure the `tokenA` will not be set twice. 

For example:
1. Owner calls `setNewTokenInfo(tokenA, tokenB)` 
2. Owner calls `setNewTokenInfo(tokenA, tokenC)`, this time the `tokenInfoList[tokenA]` has been overwritten and it may result in serious consequences if it is not intentional.

```solidity=554
    function setNewTokenInfo(address tokenA, address tokenB) public onlyOwner {
        IUniswapV2Pair _pair = IUniswapV2Pair(UniswapV2Library.pairFor(factory, tokenA, tokenB));
        //find tokenA price
        tokenInfo storage info = tokenInfoList[tokenA];
        info.pair = _pair;
        info.token0 = _pair.token0();
        info.token1 = _pair.token1();
        info.price0CumulativeLast = _pair.price0CumulativeLast();
        info.price1CumulativeLast = _pair.price1CumulativeLast();
        uint112 reserve0;
        uint112 reserve1;
        (reserve0, reserve1, info.blockTimestampLast) = _pair.getReserves();
        require(reserve0 != 0 && reserve1 != 0, "ExampleOracleSimple: NO_RESERVES");
    }
```

Potential Overwriting Risk

The function `setNewTokensInfo)()` can never be called. When executing the internal call `this.setNewTokenInfo()`, the caller will be the contract address due to the "this." feature. But the `setNewTokenInfo()` is also only allowed to call by owner, which will lead to the failure.

```solidity
    function setNewTokensInfo(address[] memory tokensA, address[] memory tokensB) external onlyOwner{
        require(tokensA.length == tokensB.length,"ERR_LENGTH_NOT_MATCH");
        for(uint i;i < tokensA.length; i++){
            this.setNewTokenInfo(tokensA[i],tokensB[i]);
        }
    }

    function setNewTokenInfo(address tokena, address tokenb) external onlyOwner {
    ...
```

Uncallable Function

When removing sort token by the function `_removeTokenFromWhitelist()`, it is not enough to check if the token is in the whitelist. The input `sort` also needs to be checked.

```solidity=45
    // remove sort token
    function _removeTokenFromWhitelist(uint sort, address token) internal {
        require(_queryIsTokenWhitelisted(token) == true, "ERR_NOT_WHITE_TOKEN");

        _tokenWhitelistedInfo[sort][token] = false;
        _isTokenWhitelisted[token] = false;
        _whiteTokenCount--;
        _countListener();

        emit LOG_DEL_WHITELIST(address(this), sort, msg.sender, token);
    }
```

Missing Validation Before Removing Sort Token

The linked functions are equivalent to the compiler-generated getter functions for the respective variables.

User-Defined Getters

The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.

Variables That Could Be Declared as Immutable

The require `require(msg.sender == _blabs, "ERR_NOT_BLABS");` validation in the functions `registerModule()`, `removeModule()` and `setOracle()` can be replaced by `modifier onlyBlabs()`.

Using Modifier `onlyBlabs` Instead

The functions which are never called internally within the contract should have external visibility for gas optimization.

Function Should Be Declared External

State variables that never change should be declared as `constant` to save gas.

State Variable Should Be Declared Constant

Accessing storage variables in a loop can be costly in terms of gas consumption.

```solidity=37
        _hasInit = true;
```


---
```solidity=39
        _whiteTokenCount++;
```


---
```solidity=51
        _whiteTokenCount--;
```


---
```solidity=59
        _whiteTokenCount == 0 ? _hasInit = false : _hasInit = true;
```


---
```solidity=57
        allOwnerPercentage += _ownerPercentage[i];
```



Costly Operation Inside Loop

One or more state variables are never used in the codebase.

Variable `_vault` in `Factory` is never used.

Variable `_initialSwapFee` in `ConfigurableRightsPool` is never used.

Variable `_publicSwap` and `_swapFee` in `LiquidityPool` are not really used, because the swap feature has been removed.

Variable `black_list`, `record_number` and `record_List` in `UserVault` are never used.

Unused State Variable

It is unnecessary to use a loop to judge the level of the fee ratio.

Loop Optimization

According to the client's response, the function `execute()` is used for future integration with other third party modules. It should be noted that this low-level method must be used very carefully. If the admin accidentally sets the malicious contract as a module contract, the hacker will be able to take all the tokens from the pool easily.

```solidity=196
    function execute(
        address _target,
        uint _value,
        bytes calldata _data
    ) external logs lock needsBPool returns (bytes memory _returnValue) {
        require(bFactory.getModuleStatus(address(this), msg.sender), "MODULE IS NOT REGISTER");

        _returnValue = bPool.execute(_target, _value, _data);
    }
```

```solidity=322
    function execute(
        address _target,
        uint _value,
        bytes calldata _data
    ) external _logs_ _lock_ returns (bytes memory _returnValue) {
        require(msg.sender == _controller, "ERR_NOT_CONTROLLER");
        require(!_finalized, "ERR_IS_FINALIZED");

        _returnValue = _target.functionCallWithValue(_data, _value);

        return _returnValue;
    }
```

The Purpose of Function `execute`

The accurate upper cap before changing is `DesynConstants.MAX_UINT`.
```solidity=252
emit sizeChanged(msg.sender, "UPPER", 0, etfStatus.upperCap);
```

Inaccurate Event Parameter

The half-year closure period should be 180 days, but it is using 30 seconds now, which is for testing only.

Test Code

According to the code logic, we suppose the performance fee is only charged when a closed-end ETF succeeds to collect enough funds and makes profit after its closed period. Typically, if a closed-end ETF fails to collect enough funds after its collected period, users will withdraw their funds directly. However, if somebody withdraws the funds from a failure pool after the closed period, which is `block.timestamp >= etfStatus.closureEndTime`, the `totalEnd` will be calculated and this may cause the performance fee being charged this time.

```solidity=428
    //perfermance fee
    uint totalEnd;
    uint profitRate;
    if (etype == SmartPoolManager.Etypes.CLOSED && block.timestamp >= etfStatus.closureEndTime) {
        totalEnd = Oracles(oracleAddress).getAllPrice(poolTokens, actualAmountsOut);
    }

    uint _actualRedeemFee = etfStatus.redeemFee;
    if (etype == SmartPoolManager.Etypes.CLOSED) {
        bool isCloseEtfCollectEndWithFailure = isCompletedCollect == false && block.timestamp >= etfStatus.collectEndTime;
        if (isCloseEtfCollectEndWithFailure) {
            _actualRedeemFee = 0;
        }
    }
```

Performance Fees that Should Not be Charged

The `testList` in the contract `DesynChainlinkOracle` seems to be just a variable which is used for testing only.

The Purpose of `testList`

There should be an array length check in the function `getAllPrice()`.

Missing Array Length Check

It is unnecessary to increase the `counters` at #L75. Currently, a number will be skipped each time a new pool being created.

```solidity=65
    function createPool() internal returns (address base) {
        bytes memory bytecode = bytecodes;
        bytes32 salt = keccak256(abi.encodePacked(counters++));

        assembly {
            base := create2(0, add(bytecode, 32), mload(bytecode), salt)
            if iszero(extcodesize(base)) {
                revert(0, 0)
            }
        }
        counters++;
    }
```

Repeated Increment

It is highly recommended NOT to include OpenZeppelin library code directly in source code, but import the original library to minimize risk because even slight changes to the library code may lead to critical/major vulnerabilities/bugs. For Solidity version 0.6.x, the latest OpenZeppelin version 3.4.2 should be used. 

OpenZeppelin Library Code Included in Source Code

There is a lot of commented code in the project. The commented code does not affect the functionality of the codebase and appears to be either remnants of test code or older functionality.

Commented Code

The `Multicall` contract cannot be set as the owner of other contracts. The function `aggregate` has no access control. If the `Multicall` contract is the owner of the `target` contract, anyone can call the privileged functions in the `target` contract.

Potential Risk of Low-level Call

There are only 6 items to add, so it is useless to init array `result[]` as `new bool[](7)`.

Inconsistent Array Length

Fund shares cannot be redeemed if they are directly traded, but the contract does not prevent users from trading directly.

```solidity=235
    uint amount2 = DesynSafeMath.bdiv(_beginFundAmount, _beginEtfAmount);
```

Irredeemable Funds

```solidity=277
    if (bools) {
        bool isCloseEtfCollectEndWithFailure = isCompletedCollect == false && block.timestamp >= collectEndTime; // fundraising failed
        bool isCloseEtfClosureEnd = block.timestamp >= closureEndTime; // `closure` end
        require(isCloseEtfCollectEndWithFailure || isCloseEtfClosureEnd, "ERR_CLOSURE_TIME_NOT_ARRIVED!");
    }
```

The `require`(Line 280) requires the following two cases:
  - block.timestamp >= collectEndTime;
  - block.timestamp >= closureEndTime;
    
None of the cases match `block.timestamp <= collectEndTime`, so the code (lines 285~286) can never be reached.

Unreachable Code 

It is unnecessary to call the function `_countListener()` inside the function `_addTokenToWhitelist()`, because the variable `_hasInit` will always be true after adding whitelist tokens.

Redundant Code

DeSyn Protocol II

Color.sol

ConfigurableRightsPool.sol

Const.sol

LiquidityPool.sol

LpToken.sol

Math.sol

Multicall.sol

Num.sol

PCToken.sol

WETH.sol

WhiteToken.sol

sorMultiCall.sol

Actions.sol

CRPFactory.sol

DSProxyContract.sol

DesynChainlinkOracle.sol

Factory.sol

Oracle.sol

UserVault.sol

Vault.sol

AggregatorV2V3Interface.sol

IBFactory.sol

IConfigurableRightsPool.sol

IERC20.sol

ILeverageStake.sol

IUniswapOracle.sol

IUniswapV2Router02.sol

IWETH.sol

Address.sol

DesynConstants.sol

DesynSafeMath.sol

RightsManager.sol

SafeApprove.sol

SafeERC20.sol

SafeMath.sol

SmartPoolManager.sol

DesynOwnable.sol

DesynReentrancyGuard.sol

Formal Verification

DeSyn Protocol is a Web3 decentralized asset management protocol that enables anyone to create and oversee tailored portfolios comprised of various on-chain assets (tokens, NFTs, derivatives, etc.) through the use of smart contracts.

DeSyn Protocol  

How Three Young Entrepreneurs Are Harnessing Social Media, Web3 And Venture Investment

Introducing Education Week: How to Learn About Web3

From DeFi to scam awareness, this site arms beginners with the tools they need to thrive in crypto - Cointelegraph

Cathie Wood: Bitcoin and Ethereum are ‘on the right track,’ despite collapse of several crypto companies

Meta’s Web3 hopes face challenge of decentralization and market headwinds

Trump’s NFT Digital Trading Cards Bring in Millions of Dollars—and GOP Ridicule - WSJ

Hermès Wins Case Against Artist Who Sold NFTs of Birkin Bags

Cathie Wood: Bitcoin and Ethereum are ‘on the right track,’ despite collapse of several crypto companies - MarketWatch

Ethereum (ETH) Sees High Volatility as U.S. Records Higher-Than-Expected GDP Growth

BTC price struggles at $21K as trader says ‘top is in’ for Bitcoin, Ethereum

DeFi Analyst Predicts $2.00 Per Token For Both Uniglo.io And Ripple In 2023 | NewsBTC

3 reasons why it could be a rocky week for Bitcoin, Ethereum and altcoins

Ethereum rallies to $1,350, but derivatives metrics remain neutral to bearish

DeFi Exchange Mango's $114M Exploit Was 'Market Manipulation,' Not a Hack, Ex-FBI Special Agent Says

2023 is off to a rough start for the web3 world as another c

Bitcoin, Ethereum, Crypto News and Price Data - CoinDesk

BNB Chain to boost European Web3 startups with DApp incubator program

Gibraltar Will Focus on Stablecoins and DeFi Lending After Market Turmoil

Critics can’t stop NFTs from becoming a mainstay of daily life

FTX.US seems to have paused withdrawals on Ethereum hours after filing for bankruptcy

"Ethereum Killer" Solana's Top Spot Threatened by FTX Drama and Emergence of Strong Opponent

Want To Know How CertiK Performs An Audit?

Want To Secure Your Wallet, Exchange or Dapp? Check Out Our Web 3 Pentesting

DeSyn Protocol

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

48

0

23

0

25

Products & Technology

Bug Bounty

Smart Contract Audit

SkyTrace

L1 Chain Audit

Incident Response

Skynet

KYC

Advisory Services

Penetration Testing

Sky Harbor

Formal Verification

Featured Ecosystems

BNB Chain

Ethereum

Avalanche

Solana

Algorand

NEAR

Cosmos

Polygon

Aptos

Company

About

Testimonials

Partnership

We're Hiring

Disclaimer

Resources

DeSyn Protocol

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

48

0

23

0

25