Read the findings of the 2022 Web3 Security Report

Contracts in this project are all upgradeable, the owner can upgrade the contract without the community's commitment. If an attacker compromises the account, he can change the implementation of the contract and drain tokens from the contract.

Centralized Control of Contract Upgrade

In the contract `BlueshiftGovernanceRouter`, the role `governor` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/c009f23b153a45898490b5205163dff6/diagrams/centralization_BlueshiftGovernanceRouter_governor.svg)

The `governor` account can create portfolios and controls the parameter settings of the `Oracle` contract and `portfolio` contract. Any compromise to the `governor` account may allow the hacker to take advantage of this authority and cause devastating consequences to the whole project. 

The `governor` account also controls the portfolio manager, who manages the LP provider's assets in the `BlueshiftLiquidityReserve`.

Following functions in contract `BlueshiftPortfolioManager` allow the `governor` account to adjust target weight in portfolio contract and sell tokens from reserves:

```solidity=
  function sellExactTokensGovernor(
        uint256 amountToSell,
        uint256 amountOutMin,
        address[] calldata path,
        uint256 deadline,
        address portfolio
    ) external onlyGovernor {
        _sellExactTokens(amountToSell, amountOutMin, path, deadline, portfolio);
    }

    function _sellExactTokens(
        uint256 amountToSell,
        uint256 amountOutMin,
        address[] calldata path,
        uint256 deadline,
        address portfolio
    ) internal returns (uint256 amountOutExpected, uint256 amountOutReal) {
        require(path.length >= 2, "ERROR: invalid path");
        (address reserveAddress1, address reserveAddress2, bool reserve2WasCreated) = _getReservesForSwap(
            path[0],
            path[path.length - 1],
            portfolio
        );
        ILiquidityReserve(reserveAddress1).transferTokens(address(this), amountToSell);
        IERC20(path[0]).approve(address(router), amountToSell);
        router.swapExactTokensForTokensSupportingFeeOnTransferTokens(amountToSell, amountOutMin, path, reserveAddress2, deadline);
        amountOutReal =
            IERC20(path[path.length - 1]).balanceOf(reserveAddress2) -
            IPortfolio(portfolio).oracle().curAmount(reserveAddress2);
        //TODO: Won`t this lead to any vulnerabilities? Should we do 2x BalanceOf() instead (one before and one after the swap)?
        if (reserve2WasCreated) {
            amountOutExpected = amountOutReal;
        } else {
            amountOutExpected = IPortfolio(portfolio).oracle().getAmountOut(reserveAddress1, reserveAddress2, amountToSell, 0);
            //TODO: We passes zero fee here, should we pass the real fee of the portfolio instead?
        }
        uint256 reserve1Price = IPortfolio(portfolio).oracle().curPrice(reserveAddress1);
        uint256 reserve2Price = ((amountToSell * reserve1Price) / amountOutReal);
        IPortfolio(portfolio).oracle().swapHappened(
            reserveAddress1,
            amountToSell,
            reserve1Price,
            reserveAddress2,
            amountOutReal,
            reserve2Price,
            false
        );
    }
```

Centralization Related Risks in `Blueshift Core`

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

File: contracts/BlueshiftPortfolio.sol (Line 184, Function `BlueshiftPortfolio.withdraw`)
```solidity
                ILiquidityReserve(reserveAddress).transferTokens(to, amounts[i]);
```


---
File: contracts/BlueshiftPortfolio.sol (Line 186, Function `BlueshiftPortfolio.withdraw`)
```solidity
                ILiquidityReserve(reserveAddress).transferTokens(msg.sender, amounts[i]);
```


---
File: contracts/BlueshiftPortfolio.sol (Line 216, Function `BlueshiftPortfolio.withdraw`)
```solidity
                reserve.transferTokens(msg.sender, amounts[i]);
```


---
File: contracts/BlueshiftPortfolio.sol (Line 219, Function `BlueshiftPortfolio.withdraw`)
```solidity
                reserve.transferTokens(to, amounts[i]);
```


---
File: contracts/BlueshiftPortfolio.sol (Line 278, Function `BlueshiftPortfolio._swap`)
```solidity
        ILiquidityReserve(reserveAddress[1]).transferTokens(to, amountOut);
```


---
File: contracts/BlueshiftPortfolioManager.sol (Line 153, Function `BlueshiftPortfolioManager._sendRewardForSwap`)
```solidity
        ILiquidityReserve(reserveAddress).transferTokens(rewardReceiver, rewardAmount);
```


---
File: contracts/BlueshiftPortfolioManager.sol (Line 170, Function `BlueshiftPortfolioManager._sellExactTokens`)
```solidity
        ILiquidityReserve(reserveAddress1).transferTokens(address(this), amountToSell);
```


---
File: contracts/BlueshiftPortfolioManager.sol (Line 171, Function `BlueshiftPortfolioManager._sellExactTokens`)
```solidity
        IERC20(path[0]).approve(address(router), amountToSell);
```



Unchecked Return Value

In the current implementation of contract `BlueshiftGoveranceRouter`, `tx.origin` is frequently used in checks.

`tx.origin` check will be no longer valid after EIP-3074 is added in the coming months. EIP3074 introduces two EVM instructions AUTH and AUTHCALL. The first sets a context variable authorized based on an ECDSA signature. The second sends a call as the authorized. This essentially delegates control of the EOA to [a] smart contract. This means there will be a way for malicious smart contracts to send transactions in the context of `owner`, thus bypassing this check.

Also, tx.origin is widely discouraged as there are possible phishing attempts.

Dangerous usage of tx.origin

In the current implementation of `BlueshiftOracle`, the function `removeReserve` is declared as an external function, which means any account can call the function to remove arbitrary reserve records from the oracle. This will lead to devastating consequences for the project if a malicious attacker deletes an active reserve from the portfolio.

```solidity=
  function removeReserve(address reserveAddress) external override {
        delete curPrice[reserveAddress];
        delete reserveStats[reserveAddress];
        delete prices[reserveAddress];
        delete reserves[reserveAddress];
        if (reserveCount > 1) {
            reserveAverageBase = (reservesTotalSupplyBase / (reserveCount - 1));
            reserveCount--;
        } else {
            delete reserveAverageBase;
            delete reserveCount;
            delete reservesTotalSupplyBase;
        }
    }
}
```

Missing access restrictions on function `removeReserve()`

In the current implementation, the fee percent of oracle can be set as much as 100%. There should be a bound that limits how high the fees can be set in the linked functions. 

Also, the checks between numerator and denominator are missing, which means `numerator/denominator` can be larger than 1. This might lead to math underflow in the `priceWithFee` calculation.

```solidity=
   function setFee(uint16 _feeNumerator, uint16 _feeDenominator) external override {
        require(
            msg.sender == portfolioFactory || msg.sender == IGovernanceRouter(governanceRouter).governor(),
            "ERROR: Only portfolio factory and governor can change fee"
        );
        feeNumerator = _feeNumerator;
        feeDenominator = _feeDenominator;
    }

    function setProtocolFee(
        uint16 _protocolFeeNumerator,
        uint16 _protocolFeeDenominator,
        address _protocolFeeReceiver
    ) external override {
        require(msg.sender == IGovernanceRouter(governanceRouter).governor(), "ERROR: Only governor can change protocol fee");
        protocolFeeNumerator = _protocolFeeNumerator;
        protocolFeeDenominator = _protocolFeeDenominator;
        protocolFeeReceiver = _protocolFeeReceiver;
    }

    function setDepositFactor(uint16 _depositFactorNumerator, uint16 _depositFactorDenominator) external override {
        require(msg.sender == IGovernanceRouter(governanceRouter).governor(), "ERROR: Only governor can change deposit factor");
        depositFactorNumerator = _depositFactorNumerator;
        depositFactorDenominator = _depositFactorDenominator;
    }

    function setWithdrawFactor(uint16 _withdrawFactorNumerator, uint16 _withdrawFactorDenominator) external override {
        require(msg.sender == IGovernanceRouter(governanceRouter).governor(), "ERROR: Only governor can change withdraw factor");
        withdrawFactorNumerator = _withdrawFactorNumerator;
        withdrawFactorDenominator = _withdrawFactorDenominator;
    }
```

Missing Restrictions on fee and factor settings

Based on our understandings, function `getAmountOut` should be a counterpart of `getAmoutIn`, while some checks in the former are missing in the latter.
For example, reserve existence checks are missing in `getAmountIn()`.

```solidity=178
  function getAmountIn(
        address reserveAddress1,
        address reserveAddress2,
        uint256 amountOut,
        uint256 gamma
    ) external view override returns (uint256 amountIn) {
        (uint256 instantReserve1, uint256 instantReserve2, ) = _getInstantReserves(
            reserveAddress1,
            reserveAddress2,
            curPrice[reserveAddress1],
            curPrice[reserveAddress2]
        );
        require(instantReserve2 > amountOut, "ERROR: Amount out exceeds reserve");
        amountIn = ((instantReserve1 * amountOut) / (((instantReserve2 - amountOut) * gamma) / Constants.feeMultiplier));
        //Ai = (ri * Bj)/(gamma * (rj - Bj))
    }

    function getAmountOut(
        address reserveAddress1,
        address reserveAddress2,
        uint256 amountIn,
        uint256 gamma
    ) external view override returns (uint256 amountOut) {
        require(reserves[reserveAddress1], "ERROR: first reserve doesn`t exist in portfolio");
        require(reserves[reserveAddress2], "ERROR: second reserve doesn`t exist in portfolio");
        (uint256 instantReserve1, uint256 instantReserve2, ) = _getInstantReserves(
            reserveAddress1,
            reserveAddress2,
            curPrice[reserveAddress1],
            curPrice[reserveAddress2]
        );
        amountOut = _getExchangeAmount(instantReserve1, instantReserve2, amountIn, gamma);
    }
```

Discrepancy between functions `getAmountIn()` and `getAmountOut()`

*This issue is considered benign because the reentrancy only acts as a double call.*

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)
File: contracts/BlueshiftOracle.sol (Line 531, Function `BlueshiftOracle.swapHappened`)
```solidity
        _takenFromReserve(reserveAddress2, amount2);
```
* This function call executes the following external call(s).
* In `BlueshiftOracle._takenFromReserve`, `ILiquidityReserve.isMain()`:
    * `curAmount[reserveAddress] == 0 && ! ILiquidityReserve(reserveAddress).isMain()`

### State variables written after the call(s)
File: contracts/BlueshiftOracle.sol (Line 452, Function `BlueshiftOracle._takenFromReserve`)
```solidity
           curPrice[reserveAddress] = 0;
```

File: contracts/BlueshiftOracle.sol (Line 533-535, Function `BlueshiftOracle.swapHappened`)
```solidity
            accumulatedProtocolFee +=
                ((amount1 * curPrice[reserveAddress1] * protocolFeeNumerator) / protocolFeeDenominator) /
                Constants.priceMultiplier;
```
There is no `reserve` existence check in the function `_takenFromReserve` and this function can be invoked in an external function call `BlueshiftPortfolioManager.sellExactTokens()`. Thus is it possible for the attacker to pass in a malicious contract address as parameter `reserveAddress`, and an external call of `isMain()` will be performed to this untrusted contract, leaving a reentrancy risk.

---

Potential reentrancy risk

The main functionalities of `_updateMA` can be described as follows:

1. The price oracle will calculate the Exponential Moving Average(ema) for each reserve price.

2. Moving average (MA) will be recalculated and updated for every swap that happened.

Financial Model risk

Variable `reserveCount` is mistakenly increased by `1` if the function parameter `token` has already been added to the reserve and oracle. Due to this issue, the variable `reserveCount` may not reflect the correct number of existing reserves, which will cause the loop between L205 and L221 in function `withdraw()` to fail.

```solidity=
  function addToken(address token, uint256 price) external override returns (RI.ReserveInfo memory reserve) {
        require(msg.sender == governanceRouter.governor() || msg.sender == governanceRouter.portfolioManager(), "ER9");
        //ERROR: Only governor can add new tokens to the portfolio
        reserve = reserveFactory.addReserve(token);
        uint256 reserveTokenPrice = reserve.isMain ? Constants.priceMultiplier : price;
        oracle.addNewReserve(reserve.reserveAddress, reserveTokenPrice);
        reserveCount++;
    }
```

State variable `ReserveCount` is updated by mistake when an existing token is added to the reserve

The contract is serving as the underlying entity to interact with third-party off-chain price oracle agents. 

According to the project Whitepaper:

> Blueshift protocol relies on external price oracles for arbitrage operations
with reduced reserves. This ensures that reduced reserves will not be possible
to use in flash loan attacks on the protocol.
For the sake of transaction fee economy and to prevent certain types of attacks, Blueshift does not use on-chain price oracles. Instead, the protocol
operation requires trusted off-chain agents that monitor token prices on external exchanges (AMM DEX, centralized exchanges, etc.). These agents
sign their price feeds using EIP-712 compatible signatures. These signatures
must be passed to the Blueshift portfolio smart contracts together with pricing data. The smart contracts can then check the validity of the signatures
and use the pricing data as external oracle prices.


The scope of the audit treats 3rd party entities as black boxes and assumes their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to malicious price feeds.

Third Party Dependencies

In function `withdraw(address[] calldata tokens, uint256[] calldata LPTamounts, address to, address WETH, uint256[] calldata minAmountOut)`, three arguments `tokens`, `LPTamounts` and `minAmountOut` should share the same length. While the current check only verifies the first two parameters and misses the check of `minAmountOut`. 

```solidity=136
  function withdraw(
        address[] calldata tokens,
        uint256[] calldata LPTamounts,
        address to,
        address WETH,
        uint256[] calldata minAmountOut
    ) external override returns (uint256[] memory amounts, uint256 WETHamount) {
        require((tokens.length == LPTamounts.length), "ER6");
```

Missing Checks for Argument

This function can be called by any account without input validation checks on parameter `path`, which will make the function `_sellExactTokens()` to perform an external call to a user-defined token contract that is untrusted.

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=170
   IERC20(path[0]).approve(address(router), amountToSell);
        router.swapExactTokensForTokensSupportingFeeOnTransferTokens(amountToSell, amountOutMin, path, reserveAddress2, deadline);
        amountOutReal =
            IERC20(path[path.length - 1]).balanceOf(reserveAddress2) -
            IPortfolio(portfolio).oracle().curAmount(reserveAddress2);
```
### State variables written after the call(s):

```solidity=
  amountOutReal =
            IERC20(path[path.length - 1]).balanceOf(reserveAddress2) -
            IPortfolio(portfolio).oracle().curAmount(reserveAddress2);
```
```solidity=
        uint256 reserve1Price = IPortfolio(portfolio).oracle().curPrice(reserveAddress1);
        uint256 reserve2Price = ((amountToSell * reserve1Price) / amountOutReal);
```

Besides, this function allows the caller to sell part of tokens from portfolio reserves (adjust weight) as long as the `weight` checks are passed in the function `_checkRealPortfolioTokensWeights`.

Potential risks in function `sellExactTokens`

The [offical solidity website](https://docs.soliditylang.org/en/v0.6.12/types.html#delete) writes that:
> delete a[x] deletes the item at index x of the array and leaves all other elements and the length of the array untouched. This especially means that it leaves a gap in the array.

In the functions `removeReserve`, deleting `reserves[reserves.length - 1]` leaves a gap. As time goes by, more and more gaps are left and the array grows larger and larger. When the array is too large, the portfolio will not be able to remove reserve because the gas limit makes it impossible to traverse the whole array.

Inaccurate Usage of operator `delete`

In the contract `MinterV2`, the role `owner` has authority over the following functions:
- function changeMultiplierFarm()
- function changeMultiplierStaking()
- function setDevPercentage()
- function setXYZ()

Any compromise to the `owner` account may allow a hacker to take advantage of this authority and modify the reward rate in farming and staking contracts.

Besides, the source of the reward token is manually added to the `MinterV2` contract and then distributed to the investors. The reward token minting process is controlled by the minter role of the `BlueshiftToken`. Users may fail to claim rewards when there is not enough balance in the `MinterV2` contract.

Centralization Related Risks in `Blueshift Farm`

Blueshift

TestERC20Token.sol

BlueshiftLPTMinter.sol

FarmingFactory.sol

BlueshiftGovernanceRouter.sol

BlueshiftOracle.sol

BlueshiftPortfolio.sol

BlueshiftPortfolioManager.sol

StakingV2.sol

BlueshiftLiquidityReserve.sol

BlueshiftPair.sol

BlueshiftPairFactory.sol

FarmingV2.sol

BlueshiftOracleFactory.sol

BlueshiftLPTMinterFactory.sol

AutoStakingV2.sol

MinterV2.sol

BlueshiftToken.sol

BlueshiftRouter.sol

Earning.sol

DefaultExternalOracle.sol

BlueshiftERC20.sol

BlueshiftPortfolioAndPairRegistry.sol

StakingFactory.sol

BlueshiftReserveFactory.sol

BlueshiftPortfolioFactory.sol

It seems that all of the `BlueShift tokens` could be sent to the same address during the creation of the currency. This could be a centralization risk as the new owner of the tokens can distribute `BlueShift tokens` without obtaining the consensus of the community.

Initial Token Distribution

Making different **one-shot-currencies** using the same `UTxO` should not be possible, hence the hardcoded UTxO here seems superfluous regarding the overall logic.

Hardcoded UTxO

The final part of the comment: 
```haskell=59
-- | Parse the UTXO from its hexadecimal string representation to and TxOutRef.
```
should be 
```haskell
... representation to a TxOutRef.
```

Blueshift

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

15

0

4

0

11