xWin Finance (Audit 2)

CertiK13 Jun 2023Files named in the report

CertiK reviewed xWIN Finance (xWin Finance (Audit 2)) on 13 Jun 2023: 66 findings, 2 critical; 57 marked fixed, 1 acknowledged, 8 open.

2 critical findings, fixed

Open the report Open xWIN Finance’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings66

Statuses are the security firm’s own, from its report.

Severity	Open	Acknowledged	Fixed
Critical	0	0	2
High	0	1	5
Medium	0	0	9
Low	8	0	41

CriticalPotential Price Manipulation Since `timeElapsed`Fixed

The `UniSwapV2TWAPOracle` contract is specifically designed to provide a fixed-window (2-hour) Uniswap V2 TWAP oracle for `xWinPriceMaster`. When the `consult()` function is called, it fetches the latest average price by calling `getLastestPrice()`. ```solidity=153 function getLastestPrice(uint256 i) internal view returns (FixedPoint.uq112x112 memory latestPrice0, FixedPoint.uq112x112 memory latestPrice1) { uint256 price0Cummulative; uint256 price1Cummulative; uint32 blockTimestamp; ( price0Cummulative, price1Cummulative, blockTimestamp ) = UniswapV2OracleLibrary.currentCumulativePrices(address(twapRegistry[i].pair)); uint32 timeElapsed = blockTimestamp - twapRegistry[i].blockTimestampLast; if (timeElapsed == 0) { latestPrice0 = twapRegistry[i].price0Average; latestPrice1 = twapRegistry[i].price1Average; } else { latestPrice0 = FixedPoint.uq112x112( uint224((price0Cummulative - twapRegistry[i].price0CummulativeLast) / timeElapsed)); latestPrice1 = FixedPoint.uq112x112( uint224((price1Cummulative - twapRegistry[i].price1CummulativeLast) / timeElapsed)); } } ``` However, it should be noted that this function retrieves the latest `price0Cumulative` and `price1Cumulative` stored in the Uniswap V2 pair and calculates the `timeElapsed` using `blockTimestamp - twapRegistry[i].blockTimestampLast`. This creates a vulnerability where an attacker can potentially manipulate the price immediately after the update of `twapRegistry[i].blockTimestampLast`, as the `timeElapsed` can be much lower than the expected 7200 seconds.

CriticalPotential Price Manipulation to `_getLPPrice()`Fixed

The `_getLPPrice()` function is used to calculate the value of an LP token in a PancakeSwap pair. This is done by calculating the value of the underlying tokens held in the pool, which in turn is used to determine the price of the LP token. The _getLPPrice function defaults to using the `token0` of the PancakeSwap pair as the underlying asset. Then it calculates the value of the LP token in the PancakeSwap pair by doubling the balance of the `token0` in the pair. This assumption may not hold true in all cases, especially if the pair is highly imbalanced. For instance, during a flash loan operation, the balance of `token0` in the liquidity pool may not reflect the actual value of the LP token since most of `token0` may be borrowed to a attacker via the `swap()` function. ```solidity=106 function _getLPPrice(address _from, address _to) internal view returns (uint rate) { uint256 decimalPlaces = 10 ** ERC20Upgradeable(_to).decimals(); address underlying = IPancakePair(_from).token0(); uint balUnderlying = IERC20Upgradeable(underlying).balanceOf(_from); uint totalSupply = IPancakePair(_from).totalSupply(); rate = getPrice(underlying, _to); if (rate == 0) { return 0; } uint token0Bal = balUnderlying * rate / decimalPlaces; rate = token0Bal * 2 * decimalPlaces / totalSupply; return rate; } ```

High`finalSwapOutput` Could Be Greater Than `swapOutpFixed

The `setPerformWithdraw` function calculates the `finalSwapOutput` when an investor initiates a withdrawal. The `finalSwapOutput` represents the number of tokens the investor obtains after deducting performance fees. However, it is possible for the `finalSwapOutput` to exceed the `swapOutput` if there are recognized and unrecognized shares due to the formula `finalSwapOutput = swapOutput - performanceUnitNoReg + (swapOutput - performanceUnitReg)`, which is marginally smaller than `2 * swapOutput`. ```solidity function setPerformWithdraw( uint256 swapOutput, uint256 _shares, uint256 _unitprice, address _investorAddress, address _managerAddress ) internal returns (uint256 finalSwapOutput) { ... if(notRecognizedShare > 0) { uint256 partialProfit = ratioNoReg * (profitPerUnit / _unitprice) * swapOutput / 10000; performanceUnit = partialProfit * actualPerformFee / 10000; finalSwapOutput = swapOutput - performanceUnit; } if(recognizedShare > 0) { uint256 ratioReg = 10000 - ratioNoReg; performanceMap[_investorAddress].shares = pM.shares - recognizedShare; uint256 anotherProfit = ratioReg * profitPerUnit / _unitprice * swapOutput / 10000; performanceUnit = performanceUnit + (anotherProfit * actualPerformFee / 10000); finalSwapOutput = finalSwapOutput + (swapOutput - performanceUnit); } ... } ```

High`pool.totalDeposit` and `pool.totalStaked` Not UFixed

Based on the design of the `emergencyWithdraw()` function in the `xWinBuddyChef` and `xWinMasterChef` contracts, users can withdraw their funds in case of an emergency without receiving any rewards. The function clears the user's deposited amount and reward debt and transfers the tokens back to the user. ```solidity user.amount = 0; user.rewardDebt = 0; pool.lpToken.safeTransfer(address(msg.sender), amount); ``` However, the values of `pool.totalStaked` in the contract `xWinBuddyChef` and `pool.totalDeposit` in the contract `xWinMasterChef` are not reduced, which will cause `pool.accCakePerShare` to become smaller, and the reward to become smaller. ```solidity uint256 lpSupply = pool.totalDeposit ... pool.accCakePerShare = pool.accCakePerShare + (cakeReward * 1e18 / lpSupply) ```

HighIncorrect Calculation of LP PriceFixed

The calculation of the price of an LP token is given as ```solidity=117 rate = token0Bal * 2 * decimalPlaces / totalSupply; ``` where `token0Bal` is the price of the `token0` tokens in the pair. The current formula assumes that the value of the `token1` tokens in the pair is equivalent to the value of the `token0` tokens, as shown by the multiplication by 2, but this is not necessarily true.

HighRugpull PotentialFixed

Each of the contracts `xWinBBMA`, `xWinDollarCostAverage`, `xWinERC20Alpha`, and `xWinIRT` contain a function `inCaseTokensGetStuck()`, allowing the owner to withdraw any token inside the contract. ```solidity function inCaseTokensGetStuck(address _token) external onlyOwner { uint256 amount = IERC20Upgradeable(_token).balanceOf(address(this)); IERC20Upgradeable(_token).safeTransfer(msg.sender, amount); } ``` However, each of the above mentioned contracts may contain certain tokens necessary for the protocol. In particular, - `xWinBBMA` may hold `stablecoin` and `targetToken` tokens - `xWinDollarCostAverage` may hold `stablecoin` tokens - `xWinERC20Alpha` may hold `_baseTokenStaking`, `baseToken`, and `targetToken` tokens - `xWinIRT` may hold `stablecoin` and `targetToken` tokens As such, certain tokens should not be allowed to be withdrawn by the owner to prevent a rugpull possibility.

HighUnrestricted Approval Exploit by Malicious Owner/Fixed

The `safeApprove` operation used in the `initialize` function and the `setSwapEngine()` function is vulnerable. The approval is given to the `swapEngine` address, which can be set arbitrarily by the owner of the contract. Similar issues in the `updateProperties()` function in some strategies. ```solidity=55 // Infinite approve IERC20Upgradeable(baseToken).safeApprove(address(swapEngine), type(uint).max); targetToken.safeApprove(address(swapEngine), type(uint).max); ``` ```solidity=303 function setSwapEngine(address _newSwapEngine) external onlyOwner { swapEngine = IxWinSwap(_newSwapEngine); if (IERC20Upgradeable(baseToken).allowance(address(this), address(swapEngine)) == 0) { IERC20Upgradeable(baseToken).safeApprove(address(swapEngine), type(uint).max); } if (targetToken.allowance(address(this), address(swapEngine)) == 0) { targetToken.safeApprove(address(swapEngine), type(uint).max); } } ``` This could allow a malicious contract owner to: 1. Set `swapEngine` to a malicious contract address using `setSwapEngine()` or update the `swapEngine` contract. 2. Exploit the infinite approval granted to the `swapEngine` to **drain all the tokens in the pool**.

Medium`amount` Could Be Smaller Than `DEFAULT_CREATION_Fixed

In the `createFundPrivate()` function, the first condition checks if the caller is whitelisted. If not, it requires that the caller has enough `xWinToken` balance to pay the default creation fee, which is set as `DEFAULT_CREATION_FEE`. If the caller has enough xWinToken balance, it transfers the amount specified in the `amount` argument from the caller's address to `xWinAdminWallet`. However, the code allows the caller to transfer any amount of `xWinToken` to `xWinAdminWallet`, which may be less than the `DEFAULT_CREATION_FEE`. In this case, the caller would be able to create a fund without paying the full creation fee. ```solidity=90 function createFundPrivate( uint256 amount, string memory name, string memory symbol, address[] calldata _toAddresses, uint256[] calldata _targetWeight ) external { if(!whitelisted[msg.sender]){ require(xWinToken.balanceOf(msg.sender) >= DEFAULT_CREATION_FEE, "not enough xwin"); xWinToken.safeTransferFrom(msg.sender, xWinAdminWallet, amount); } ... } ```

Medium`baseCcyAmt` Counted TwiceFixed

The `emergencyRedeem()` function allows users to redeem their tokens in the case of an emergency or when the contract is paused. It will first calculate amount of base currency they will receive based on the `redeeemratio`. Then there is a for loop that iterates through an array of addresses called `targetAddr` and calculates the amount of `targetAddr[i]` users will receive. ```solidity=265 function emergencyRedeem(uint256 redeemUnit) external whenPaused { ... uint256 totalOutput = redeemratio * getBalance(baseCcy) / 1e18; IERC20Upgradeable(baseCcy).safeTransfer(msg.sender, totalOutput); for (uint256 i = 0; i < targetAddr.length; i++) { xWinLib.transferData memory _transferData = _getTransferAmt(targetAddr[i], redeemratio); if(_transferData.totalTrfAmt > 0){ IERC20Upgradeable(targetAddr[i]).safeTransfer(msg.sender, _transferData.totalTrfAmt); } } } /// @dev Calc return balance during redemption function _getTransferAmt(address underying, uint256 redeemratio) ... if (underying == baseCcy) { _transferData.totalUnderlying = baseCcyAmt; } else { _transferData.totalUnderlying = getBalance(underying); } uint256 qtyToTrf = redeemratio * _transferData.totalUnderlying / 1e18; _transferData.totalTrfAmt = qtyToTrf; return _transferData; } ``` However, If `baseCcy` is included in the `targetAddr` array, the amount of `baseCcyAmt` will be counted twice, allowing the user to receive more `baseCcy` tokens than expected. This can also prevent future users from withdrawing due to having insufficient `baseCcy` tokens.

Medium`baseCcyAmt` Not Decreased During Emergency RedeeFixed

If `baseCcy` is in the `targetAddr` array, then `baseCcyAmt` represents the amount of `baseCcy` tokens that are meant for investment. When a user redeems shares, they obtain some amount of tokens in each of `targetAddr` depending on the amount of shares. ```solidity=268 for (uint256 i = 0; i < targetAddr.length; i++) { xWinLib.transferData memory _transferData = _getTransferAmt(targetAddr[i], redeemratio); if(_transferData.totalTrfAmt > 0){ IERC20Upgradeable(targetAddr[i]).safeTransfer(msg.sender, _transferData.totalTrfAmt); } ``` If `targetAddr[i] == baseCcy`, then the amount of `baseCcy` tokens is based on `baseCcyAmt`. ```solidity=281 if (underying == baseCcy) { _transferData.totalUnderlying = baseCcyAmt; } else { _transferData.totalUnderlying = getBalance(underying); } uint256 qtyToTrf = redeemratio * _transferData.totalUnderlying / 1e18; ``` However, `baseCcyAmt` is never reduced after the redemption, meaning the value is inflated, causing an inaccurate investment portfolio.

Show 10 more

Medium`block.number` Could Be Greater Than `pool.endPerFixed

In the `updatePool()` function, the `multiplier` is calculated using `pool.lastRewardBlock` and `block.number`. However, the `block.number` variable could be greater than `pool.endPeriod`, resulting in more rewards. ```solidity function updatePool(uint256 _pid) public { ... uint256 multiplier = getMultiplier(pool.lastRewardBlock, block.number); uint256 cakeReward = multiplier * xwinPerBlock * pool.allocPoint / totalAllocPoint; ... } ```

MediumFailure to Initialize ContractsFixed

The `__xWinStrategyInteractor_init()` and `__xWinStrategy_init()` functions have the modifier `initializer`, which will conflict with the `initialize()` functions in `xWinSwap` and other strategies contracts since the initializer will be executed twice when calling the `initialize()` function in the sub-contracts. *Initializable.sol:* ```solidity=83 modifier initializer() { bool isTopLevelCall = !_initializing; require( (isTopLevelCall && _initialized < 1) || (!AddressUpgradeable.isContract(address(this)) && _initialized == 1), "Initializable: contract is already initialized" ); _initialized = 1; if (isTopLevelCall) { _initializing = true; } _; if (isTopLevelCall) { _initializing = false; emit Initialized(1); } } ```

MediumInaccurate `amountOut` And Incorrect `swapExactToFixed

The `_swapTokenToToken()` function is used for swapping tokens on the UniswapV2-like decentralized exchange. If the input token is a supporting fee-on-transfer token (such as SafeMoon), the function should use the `swapExactTokensForTokensSupportingFeeOnTransferTokens()` function to perform the swap. This function swaps the input token for the output token and sends the output token to the destination address. ```solidity=51 uint256[] memory amounts = swapRouter.getAmountsOut(amountIn, path); uint256 amountOut = amounts[amounts.length-1]; if(useSupportingFeeOnTransferTokens[path[0]]){ swapRouter.swapExactTokensForETHSupportingFeeOnTransferTokens( amountIn, amountIn + (amountIn * slippage / 10000), path, destination, block.timestamp ); return amounts[amounts.length-1]; ``` However, the function mistakenly uses the `swapExactTokensForETHSupportingFeeOnTransferTokens()` function instead, which is used to swap an exact amount of tokens for as much ETH as possible. Moreover, the use of `amounts[amounts.length-1]` to determine the amount of output tokens received after the swap is inaccurate because the `getAmountsOut()` function fails to account for fee-on-transfer tokens. Also, the second input uses `amountIn` when it should be based on `amountOut`. ```solidity function swapExactTokensForTokensSupportingFeeOnTransferTokens( uint amountIn, uint amountOutMin, address[] calldata path, address to, uint deadline ) external virtual override ensure(deadline) { TransferHelper.safeTransferFrom( path[0], msg.sender, UniswapV2Library.pairFor(factory, path[0], path[1]), amountIn ); uint balanceBefore = IERC20(path[path.length - 1]).balanceOf(to); _swapSupportingFeeOnTransferTokens(path, to); require( IERC20(path[path.length - 1]).balanceOf(to).sub(balanceBefore) >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT' ); } ```

MediumInconsistent Total Supply CalculationFixed

In the `withdraw()`, `getUnitPrice()`, `_getUnitPrice()` and `getUnitPriceInUSD()()` functions, the code uses `totalSupply()` to calculate the unit price and the amount to withdraw. However, it should use `getFundTotalSupply()` to calculate these values. In the `getFundTotalSupply()` method, the total supply is calculated by adding the current `totalSupply()` and `pendingMFee`. Therefore, the functions like `getUnitPrice()` and `withdraw()` are currently underestimating the total supply, which can result in an incorrect calculation of the unit price and the amount to withdraw. Similar issues occur in other strategies that support fees. ```solidity function getFundTotalSupply() public virtual view returns(uint256) { return totalSupply() + pendingMFee; } ``` ```solidity function getUnitPrice() public view override returns (uint256) { return _convertTo18(_getUnitPrice(), baseToken); } ``` ```solidity function withdraw(uint _amount) external override nonReentrant whenNotPaused returns (uint256) { require(_amount > 0, "Nothing to withdraw"); require(_amount <= IERC20Upgradeable(address(this)).balanceOf(msg.sender), "Withdraw amount exceeds balance"); // swap any comp available into target token so that it is included into fund total values uint256 redeemratio = _amount * 1e18 / totalSupply(); ```

MediumPossible Underflow Revert Caused By TransferringFixed

The `performanceMap[msg.sender].shares` specifically represents the number of shares of the fund currently held by the investor whose address is `msg.sender`. The shares field keeps track of each investor's share ownership in the fund and is updated whenever they deposit or withdraw from the fund. The `shares` field will be used for calculating the performance fee when the user withdraws from the fund. ```solidity function deposit(uint256 amount) override external whenNotPaused returns (uint256){ ... setPerformDeposit(mintQty, unitPrice); ... } function setPerformDeposit(uint256 mintShares, uint256 latestUP) internal { uint256 newTotalShares = performanceMap[msg.sender].shares + mintShares; performanceMap[msg.sender].avgPrice = ((performanceMap[msg.sender].shares * performanceMap[msg.sender].avgPrice) + (mintShares * latestUP)) / newTotalShares; performanceMap[msg.sender].shares = newTotalShares; } function withdraw(uint256 amount) override external whenNotPaused returns (uint256){ ... uint256 finalOutput = setPerformWithdraw(totalOutput, amount, unitP, msg.sender, managerAddr); ... } ... function setPerformWithdraw( uint256 swapOutput, uint256 _shares, uint256 _unitprice, address _investorAddress, address _managerAddress ) internal returns (uint256 finalSwapOutput){ ... if(performFee == 0 || waivedPerformanceFees[msg.sender] || profitPerUnit == 0){ uint remain = pM.shares - _shares; ... } ... } ``` However, If the performance fee is set to zero, `msg.sender` is waived performance fees, or the profit per unit is 0, and `msg.sender` received the shares from someone transferring the shares to them without updating the `performanceMap`, then the user will be unable to withdraw due to underflow revert in the calculation of `uint remain = pM.shares - _shares`. Moreover, if the swap contract is exempt from fees, this may prevent calling the `withdraw()` function to swap from strategy tokens.

MediumUninitialized `lastManagerFeeCollection`Fixed

The `lastManagerFeeCollection` variable represents the last block number at which the manager fee was collected. However, it's not initialized in the `__xWinStrategyWithFee_init()` function. In the `_calcFundFee()` function, the `totalblock` is calculated by subtracting `lastManagerFeeCollection` from the current block number. If `lastManagerFeeCollection` is uninitialized (which would default to 0), then the first calculation of `totalblock` would take into consideration all the blocks since the blockchain started, which would vastly overestimate the amount of fee to be collected. ```solidity=59 function _calcFundFee() internal virtual { uint256 totalblock = block.number - lastManagerFeeCollection; lastManagerFeeCollection = block.number; uint256 supply = getFundTotalSupply(); ```

Low`swapEngine` Should Be Authorized Within `setSwapFixed

The `swapEngine` contract should be authorized immediately after it has been set in the `setSwapEngine()` function to prevent unintended reverts when calling the `deposit()` and `withdraw()`. Additionally, the audit team recommends that clients revoke authorization for the previous `swapEngine` to prevent any inconsistencies between the old and new versions. ```solidity function setAuthourised(address _newAddr, bool _allow) external onlyOwner { authourised[_newAddr] = _allow; } function setSwapEngine(address _newSwapEngine) external onlyOwner { swapEngine = IxWinSwap(_newSwapEngine); ... } ```

Low`tokenPerBlock * duration * blocksPerDay` May NotFixed

The `set()` function will update the `endPeriod` and `tokenPerBlock`. However, these two variables may be inconsistent with the `totalBuddyTokens` variable, which is initialized in the `add()` function. ```solidity function add(IERC20Upgradeable _lpToken, IERC20Upgradeable _rewardToken, bool _withUpdate, uint _duration, uint _totalBuddyTokens) public onlyOwner { ... tokenPerBlock: _totalBuddyTokens / _duration / blocksPerDay, totalBuddyTokens: _totalBuddyTokens, ... } function set(uint _pid, bool _withUpdate, uint _newDuration, uint _tokenPerBlock) public onlyOwner { if (_withUpdate) { massUpdatePools(); } poolInfo[_pid].endPeriod = _newDuration * blocksPerDay + block.number; poolInfo[_pid].tokenPerBlock = _tokenPerBlock; } ```

Low`xWinSwap` Contract Should Be Exempted From PerfoFixed

The `xWinSwap` contract allows users quickly deposit and withdraw from xWin strategies. However, according to the logic in the `FundV2` contract, the performance fees are applied differently for recognized and non-recognized shares, where fees are higher for non-recognized shares. If the swap contract was not fee exempt, it will be hard to tell when the shares will be recognized or non-recognized, leading to inaccurate fees.

LowHard-coded `blocksPerDay` In `GetAPR()`Fixed

In the `getAPR()` function, `blocksPerDay` is hard-coded as 28800, which could be inaccurate if the owner changes the `blocksPerDay` via `setBlockPerDay()`.

Show 10 more

LowIncompatibility With Deflationary TokensFixed

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee) to the target contract, only 90 tokens actually arrive to the contract. Several strategies use the function `swapTokenToToken()`, which will transfer `_amount` tokens from the strategy contract to `xWinSwap` and then `xWinSwap` will attempt to swap `_amount` tokens. If the token was deflationary, `xWinSwap` would receive less than `_amount`, possibly causing the swap to fail.

LowInconsistency With CommentsFixed

The comments in the `xWinIRT` and `xWinBBMA` contracts indicate the `systemReTrade()` function will be triggered by chainlink keeper when there is a remaining buy or close-buy. However, there is no implementation for this feature. The comments in the `xWinLockedStake` contract indicate the deposit operation can only be executed when the contract is not paused, but there is no `whenNotPaused` modifier for this function.

LowInconsistent `decimalPlaces` in `getPrice()`Fixed

In the `getPrice()` function, the `flip` variable in the code is a boolean variable that is used to determine whether or not the exchange rate between the two tokens needs to be flipped after it has been calculated. The exchange rate is calculated based on the input tokens `_from` and `_to`. If there is a direct mapping for these tokens in the `priceSourceMap`, then the exchange rate is calculated based on that mapping. However, if there is no direct mapping, the function checks if there is a reverse mapping (`_to` to `_from`) in the `priceSourceMap`. ```solidity=47 function getPrice(address _from, address _to) public view returns (uint rate){ bool flip = false; // if both from and to are same, the exrate is 1 uint256 decimalPlaces = 10 ** ERC20Upgradeable(_to).decimals(); if(_from == _to) return decimalPlaces; SourceMap memory map = priceSourceMap[_from][_to]; if(map.source == 0){ map = priceSourceMap[_to][_from]; address tempFrom = _from; _from = _to; _to = tempFrom; flip = true; // no map source defined. revert require(map.source != 0, "PriceMaster: unsupported input addresses"); } ... if(flip) rate = decimalPlaces * decimalPlaces / rate; ... } ``` The `decimalPlaces` variable represents the decimals for the original `_to` token before the swap. However, after the swap and the exchange rate flip, the `rate` variable now represents the exchange rate in terms of the flipped `_to` token, which may have a different number of decimal places. If the decimals of the flipped `_to` token is greater than the `decimalPlaces` variable, then the resulting exchange rate will be truncated to zero due to integer division. For instance, let's consider a scenario where the `decimalPlaces` is set to $10^6$, and the decimals of the rate for the flipped `_to` token is $10^{18}$. In this case, the exchange rate will be truncated to zero due to integer division by $\frac{10^6 \times 10^6}{10^{18}}$.

LowInconsistent Token Used for Decimals Between `_geFixed

The calculations for `accruedCompInUSD` in `_getVaultValuesInUSD()` and `canReclaimRainMaker()` may not be consistent if `targetToken` has a different number of decimals than `stablecoinUSDAddr`. ```solidity function _getVaultValuesInUSD() internal view returns (uint256){ ... uint exRateLendingBase = xWinPriceMaster.getPrice(address(lendingRewardToken), stablecoinUSDAddr); ... uint accruedComp = getAccruedComp(); uint accruedCompInUSD = exRateLendingBase * accruedComp / getDecimals(address(targetToken)); ... } ``` ```solidity function canReclaimRainMaker() public view returns (bool) { ... uint accruedComp = getAccruedComp(); ... uint exRateLendingBase = xWinPriceMaster.getPrice(address(lendingRewardToken), stablecoinUSDAddr); uint accruedCompInUSD = exRateLendingBase * accruedComp / getDecimals(address(stablecoinUSDAddr)); ... } ```

LowIncorrect FundV2 functionsFixed

The `FundV2Factory` contract is missing three functions to update state variables in the `FundV2` contract. Additionally, it contains two updating functions that call nonexistent functions in the `FundV2` contract. Missing: - updateLockedStakingAddress() - addContractWaiveFee() - removeContractWaiveFee() Functionality that does not exist in FundV2: - updateWETH()

LowIncorrect Return ValueFixed

The function `_buyUnderWeightNames()` uses `baseCcy` tokens to swap to target tokens that are under-invested in the portfolio. ```solidity=354 function _buyUnderWeightNames (xWinLib.UnderWeightData[] memory underweights, uint256 totalunderwgt) internal returns (uint256 baseccyBal) { baseccyBal = getBalance(baseCcy) - baseCcyAmt; for (uint256 i = 0; i < underweights.length; i++) { if(underweights[i].token != address(0)){ uint256 rebBuyQty = underweights[i].activeWeight * baseccyBal / totalunderwgt; if(rebBuyQty > 0 && rebBuyQty <= baseccyBal){ xWinSwap.swapTokenToToken(rebBuyQty, baseCcy, underweights[i].token); if(underweights[i].token == baseCcy) { baseCcyAmt += rebBuyQty; } } } } return baseccyBal; ``` The return value of this function is `baseccyBal`, which is assigned the contract's `baseCcy` balance before swapping to underweight tokens. Since buying swapping for underweight tokens will reduce the contract's `baseCcy` balance, the current return value may be incorrect.

LowInefficient Memory and Storage ParameterFixed

One or more parameters with `memory` data location are never modified in their functions and those functions are never called internally within the contract. Thus, their data location can be changed to `calldata` to avoid the gas consumption copying from calldata to memory. ```solidity=56 function initialize( ``` `initialize` has memory location parameters: `_name`, `_symbol`. --- ```solidity=31 function initialize( ``` `initialize` has memory location parameters: `_name`, `_symbol`. --- ```solidity=76 function initialize(string memory name, string memory symbol, address _usdtToken, uint256 _blocksPerDay) initializer public { ``` `initialize` has memory location parameters: `name`, `symbol`. --- ```solidity=35 function initialize( ``` `initialize` has memory location parameters: `name`, `symbol`. --- ```solidity=35 function initialize( ``` `initialize` has memory location parameters: `name`, `symbol`. --- ```solidity=932 function addxwinFund(address[] calldata _fundaddress, bool [] memory _isxwinFund) public onlyOwner { ``` `addxwinFund` has memory location parameters: `_isxwinFund`. --- ```solidity=1050 function Subscribe(xWinLib.TradeParams memory _tradeParams) public nonReentrant onlyNonEmergency payable { ``` `Subscribe` has memory location parameters: `_tradeParams`. --- ```solidity=38 function FeeEvent(string memory _eventtype, address _contractaddress, uint256 _fee) public onlyExecutor { ``` `FeeEvent` has memory location parameters: `_eventtype`. --- ```solidity=42 function FundEvent( ``` `FundEvent` has memory location parameters: `_type`. --- ```solidity=21 function __xWinStrategy_init( ``` `__xWinStrategy_init` has memory location parameters: `name`, `symbol`. --- ```solidity=174 function addTokenPath( ``` `addTokenPath` has memory location parameters: `path`. --- ```solidity=172 PoolInfo storage pool = poolInfo[_pid]; UserInfo storage user = userInfo[_pid][_user]; ``` `pool` and `user` can be changed to `memory`

LowInsufficient Cases In `_xWinStratSwap()`Fixed

The `_xWinStratSwap()` function handles two cases involving xWin strategy tokens: the first is when the token being swapped from is an xWin strategy token and the second is when the token being swapped to is an xWin strategy token. However, the function does not account for the scenario where both tokens are xWin strategy tokens. ```solidity function _xWinStratSwap(uint256 _amount, address _fromToken, address _toToken) internal returns(bool, uint) { if (isxWinStrategy(_fromToken)) { ... } else if (isxWinStrategy(_toToken)) { ... } else { return (false, 0); } } ```

LowLack Of Input Validation for `_fromtoken`, `_totoFixed

The `addTokenPath()` function is used to store token conversion path information for the `xWinSwap` contract. It does not include any input validation for `_fromtoken`, `_totoken`, and `path`. For example, the `path` must start with `_fromtoken` and end with `_totoken`. ```solidity=174 function addTokenPath( address _router, address _fromtoken, address _totoken, address[] memory path, uint256 _slippage ) public onlyExecutor { PathInfo storage pathInfo_ = pathInfoRouter[_fromtoken][_totoken]; address[] memory realPath = new address[](path.length); address[] memory _inversePath = new address[](path.length); for (uint256 i; i < path.length; i++) { realPath[i] = path[i]; _inversePath[path.length-i-1] = realPath[i]; } pathInfo_.fromToken = _fromtoken; pathInfo_.toToken = _totoken; pathInfo_.path = realPath; pathInfo_.inversePath = _inversePath; pathInfo_.bestRouter = _router; pathInfo_.slippage = _slippage; } ```

LowLack of Storage Gap in Upgradeable ContractFixed

There is no storage gap preserved in the **all-upgradable contracts** in the xWin Finance Project. Any logic contract that acts as a base contract that needs to be inherited by other upgradeable child should have a reasonable size of storage gap preserved for the new state variable introduced by the future upgrades.

Show 10 more

LowLack of Validation For `targetAddr` and `toAddr`Fixed

The `createTargetNames()` function is designed to update the target names and their respective weights to be used in the contract's rebalancing strategy. The function first clears the weights of the target addresses to zero and deletes all elements of `targetAddr`. Then the function loops through each element of `_toAddr` and sets the weight of the target address `_toAddr[i]` to `_targets[i]`. ```solidity=138 function createTargetNames(address[] calldata _toAddr, uint256[] calldata _targets) public onlyRebManager { uint256 sum = sumArray(_targets); require(sum == 10000, "xWinFundV2: Sum must equal 100%"); if (targetAddr.length > 0) { for (uint256 i = 0; i < targetAddr.length; i++) { _getLatestPrice(targetAddr[i]); // ensures that the address provided is supported TargetWeight[targetAddr[i]] = 0; } delete targetAddr; } for (uint256 i = 0; i < _toAddr.length; i++) { TargetWeight[_toAddr[i]] = _targets[i]; targetAddr.push(_toAddr[i]); } _approveAll(); } ``` However, although the comment shows calling the `_getLatestPrice` function to ensure that the address provided is supported, the function only validates the addresses in the previous `targetAddr`. Instead, the addresses in `_toAddr` should be validated. Furthermore, the length of both `_toAddr` and `_targets` should be checked to ensure that they are the same and to prevent unintended revert. Finally, the function should ensure that there are no duplicate addresses in `_toAddr`.

LowLocked EtherFixed

The contract has one or more payable functions, but does not have a function to withdraw the fund. ```solidity=75 function depositToStrategy(uint256 _amount, address _strat) public payable returns (uint256) { ```

LowMissing Error Handling for Uninitialized `swapDatFixed

The `internalSwap()` function retrieves `SwapInfo` from `swapData` for a given token pair. However, there is no guarantee that `SwapInfo` has been properly initialized for this pair. If the SwapInfo is not initialized, the function may use default or null values, potentially leading to unexpected behavior or revert. ```solidity=102 function internalSwap( uint _amount, address _fromToken, address _toToken, address recipient, uint32 _slippage ) internal returns (uint) { uint amountsOut = 0; SwapInfo memory d = swapData[_fromToken][_toToken]; ```

LowMissing Input Validation for `uint[] ratio`Fixed

In the `initialize()` and `restateManagers()` functions, the contract assumes that all managers have the same length for their ratio arrays. This assumption can lead to logical bugs and even potential vulnerabilities. For instance, if `_managers[0]` have more ratio entries than others, the code could try to access an array element that doesn't exist. This would cause the transaction to revert. ```solidity=29 for(uint j = 0; j < _managers[0].ratio.length; j++) { uint sum; for(uint i = 0 ; i < _managers.length; i++){ sum = sum + _managers[i].ratio[j]; } require(sum == 10000, "Ratios must be equal to 100%"); } ```

LowMissing Zero Address ValidationFixed

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=39 WBNB = _WBNB; ``` - `_WBNB` is not zero-checked before being used. --- ```solidity=66 managerAddr = _manageraddr; ``` - `_manageraddr` is not zero-checked before being used. --- ```solidity=67 managerRebAddr = _managerRebaddr; ``` - `_managerRebaddr` is not zero-checked before being used. --- ```solidity=68 platformWallet = _platformWallet; ``` - `_platformWallet` is not zero-checked before being used. --- ```solidity=69 lockingAddress = _lockedStaking; ``` - `_lockedStaking` is not zero-checked before being used. --- ```solidity=90 baseCcy = _baseCcy; ``` - `_baseCcy` is not zero-checked before being used. --- ```solidity=613 platformWallet = _newAddr; ``` - `_newAddr` is not zero-checked before being used. --- ```solidity=625 managerAddr = newManager; ``` - `newManager` is not zero-checked before being used. --- ```solidity=626 managerRebAddr = newRebManager; ``` - `newRebManager` is not zero-checked before being used. --- ```solidity=637 lockingAddress = _lockedStaking; ``` - `_lockedStaking` is not zero-checked before being used. --- ```solidity=72 xWinAdminWallet = _xWinAdminWallet; ``` - `_xWinAdminWallet` is not zero-checked before being used. --- ```solidity=73 xWinSwapAddr = _xWinSwapAddr; ``` - `_xWinSwapAddr` is not zero-checked before being used. --- ```solidity=75 autoLockAddr = _xWinLockStaking; ``` - `_xWinLockStaking` is not zero-checked before being used. --- ```solidity=77 beaconContract = _beaconAddress; ``` - `_beaconAddress` is not zero-checked before being used. --- ```solidity=78 DEFAULT_BASETOKEN = _baseToken; ``` - `_baseToken` is not zero-checked before being used. --- ```solidity=79 DEFAULT_MANAGER = _defaultManagerAddr; ``` - `_defaultManagerAddr` is not zero-checked before being used. --- ```solidity=83 DEFAULT_PLATFORM = _defaultPlatformAddr; ``` - `_defaultPlatformAddr` is not zero-checked before being used. --- ```solidity=229 xWinSwapAddr = _xWinSwapAddr; ``` - `_xWinSwapAddr` is not zero-checked before being used. --- ```solidity=264 DEFAULT_BASETOKEN = _baseToken; ``` - `_baseToken` is not zero-checked before being used. --- ```solidity=267 DEFAULT_MANAGER = _defaultManager; ``` - `_defaultManager` is not zero-checked before being used. --- ```solidity=329 xWinAdminWallet = _address; ``` - `_address` is not zero-checked before being used. --- ```solidity=337 autoLockAddr = _newAutoLock; ``` - `_newAutoLock` is not zero-checked before being used. --- ```solidity=53 devAddress = _devAddress; ``` - `_devAddress` is not zero-checked before being used. --- ```solidity=56 usdtToken = _usdtToken; ``` - `_usdtToken` is not zero-checked before being used. --- ```solidity=73 devAddress = _devAdd; ``` - `_devAdd` is not zero-checked before being used. --- ```solidity=274 usdtToken = _newUSDAddr; ``` - `_newUSDAddr` is not zero-checked before being used. --- ```solidity=68 treasury = _treasury; ``` - `_treasury` is not zero-checked before being used. --- ```solidity=83 usdtToken = _usdtToken; ``` - `_usdtToken` is not zero-checked before being used. --- ```solidity=343 usdtToken = _newUSDToken; ``` - `_newUSDToken` is not zero-checked before being used. --- ```solidity=919 platformWallet = _platformWallet; ``` - `_platformWallet` is not zero-checked before being used. --- ```solidity=920 xwinBenefitPool = _xwinBenefitPool; ``` - `_xwinBenefitPool` is not zero-checked before being used. --- ```solidity=924 stakeAddress = _stakeAddress; ``` - `_stakeAddress` is not zero-checked before being used. --- ```solidity=925 xWinToken = _xWinToken; ``` - `_xWinToken` is not zero-checked before being used. --- ```solidity=1164 xwinBenefitPool = _xwinBenefitPool; ``` - `_xwinBenefitPool` is not zero-checked before being used. --- ```solidity=1174 stakeAddress = newStakeProtocol; ``` - `newStakeProtocol` is not zero-checked before being used. --- ```solidity=43 _TWAPOracle = _twapAddress; ``` - `_twapAddress` is not zero-checked before being used. --- ```solidity=150 _TWAPOracle = _address; ``` - `_address` is not zero-checked before being used. --- ```solidity=30 stablecoinUSDAddr = _USDTokenAddr; ``` - `_USDTokenAddr` is not zero-checked before being used. --- ```solidity=55 stablecoinUSDAddr = _newUSDAddr; ``` - `_newUSDAddr` is not zero-checked before being used. --- ```solidity=45 data.baseToken = _baseToken; ``` - `_baseToken` is not zero-checked before being used.

LowNo Upper Limit in FeeRateFixed

The current implementation has no upper limit for the fee rate. Misuse of these fee-setting functions could damage the whole logic of contracts. For instance, the owner of the contract could set the fee rate to over 100%, which probably would result in all transactions being reverted.

LowOrigin of RewardsFixed

According to the design of the `xWinBuddyChef` contract, the admin can create a pool for a pair of `lpToken` and `rewardToken`. Users can deposit `lpToken` to it and get `rewardToken` and `lpToken` back. However, no explicit function or design guarantees the availability of `rewardToken`. The audit team would like to confirm the origin of the rewards. Also, the current implementation of the `add()` function does not prevent the `lpToken` from being the same as the `rewardToken`, which could lead to a problematic situation or create a Ponzi-like scheme. The audit team recommends clients add extra validation.

LowPool Is Not Updated When Stopping ItFixed

A pool can be stopped through the `stopPool()` function, but the pool is not updated when using this function. ```solidity=123 function stopPool(uint256 _pid) external onlyOwner { _stopPool(_pid); } function _stopPool(uint256 _pid) internal { uint256 prevAllocPoint = poolInfo[_pid].allocPoint; poolInfo[_pid].allocPoint = 0; poolInfo[_pid].endPeriod = block.number; poolInfo[_pid].lastRewardBlock = block.number; totalAllocPoint = totalAllocPoint - prevAllocPoint; } ``` As stopping a pool changes its `allocPoint` to 0, rewards will not be updated properly.

LowPossibly No Fees For Unrecognized SharesFixed

For unrecognized shares, the calculation of their withdraw fees are as follows: ```solidity=679 if(notRecognizedShare > 0) { uint256 partialProfit = ratioNoReg * (profitPerUnit / _unitprice) * swapOutput / 10000; performanceUnit = partialProfit * actualPerformFee / 10000; finalSwapOutput = swapOutput - performanceUnit; } ``` The value of `profitPerUnit` is given below: ```solidity=658 uint256 profitPerUnit = _unitprice > pM.avgPrice ? _unitprice - pM.avgPrice : 0; ``` If `_unitprice > pM.avgPrice`, then `profitPerUnit = _unitprice - pM.avgPrice <= _unitprice`. This means `profitPerUnit / _unitprice <= 1` at all times and will be zero if `pM.avgPrice > 0`. Since the calculation of `partialProfit` first computes `profitPerUnit / _unitprice`, a user can cause `partialProfit == 0` by ensuring their `pM.avgPrice > 0`, such as buying 1 share. This allows the user to withdraw any unrecognized shares without paying any fees.

LowPotential Assets Loss After Calling `setMasterCheFixed

The `setMasterChef()` and `setMasterChefPids()` functions are used to set the master chef contract address and the pool IDs. When calling these two functions, it is possible that the assets may be at risk of loss if these functions do not withdraw them from the old contract before migrating to the new one. ```solidity=228 function setMasterChef(address _newMasterChef) external onlyOwner { masterChef = xWinMasterChef(_newMasterChef); IERC20Upgradeable(token).safeApprove(_newMasterChef, type(uint).max); } function setMasterChefPids(uint256 _xWinPid, uint256 _lockPid) external onlyOwner{ xwinpid = _xWinPid; lockpid = _lockPid; } ```

Show 10 more

LowPotential Back-Running Transaction By Manager AllFixed

In the `xWinStrategyWithFee` contract, the `totalProfit` is calculated using the formula `(unitPrice - watermarkUnitprice) * totalUnits / 1e18`. Here, `totalUnits` is based on `getFundTotalSupply()`, which means a higher total supply results in a greater performance fee. Hence, once the managers are able to collect the performance fee, they can patiently await a large deposit of base tokens into the pool. After the user completes the deposit transaction, the managers can execute a back-running transaction to call `collectPerformanceFee()`, resulting in the immediate loss of a portion of the user's deposit. ```solidity=85 if (watermarkUnitprice < unitPrice) { uint totalUnits = getFundTotalSupply(); uint totalProfit = (unitPrice - watermarkUnitprice) * totalUnits / 1e18; // collect performance fee uint feeAmt = totalProfit * performanceFee / 10000; uint newShares = _getMintQty(feeAmt); _mint(strategyManager, newShares); watermarkUnitprice = unitPrice; } ```

LowPotential Inconsistency Between `maxPerSwap` andFixed

The `maxPerSwap` variable restricts the quantity of `stalecoin` tokens that can be swapped and is initialized with `1000 ether`. However, this value may be incompatible with the decimals of the `stablecoin` token.

LowPotential Inconsistent Decimal In `getAmountToSwaFixed

The `getAmountToSwap()` function calculates the amount of `baseToken` that needs to be withdrawn from the `_baseTokenStaking` contract. To calculate `olaStableBal`, it seems to assume the decimals of `_baseTOkenStaking` is 18. To prevent potential inconsistency and optimize the code, the audit team recommends calculating the `olaStableBal` by `uprice * unitOwn / getDecimals(address(_baseTokenStaking))` ```solidity=116 function getAmountToSwap() public view returns (uint){ // total stable coin staked in the single asset uint unitOwn = _baseTokenStaking.balanceOf(address(this)); uint uprice = xWinPriceMaster.getPrice(address(_baseTokenStaking), address(baseToken)); uint olaStableBal = _convertFrom18(uprice * unitOwn / 1e18, address(baseToken)); uint tobeinvestInAmount = olaStableBal > totalDeposit ? olaStableBal - totalDeposit : 0; uint tobeinvestInUnit = tobeinvestInAmount * 1e18 / uprice; return tobeinvestInUnit; } ```

LowPotential Inconsistent Decimals Between `baseTokeFixed

In the `getVaultValuesInUSD()` function, all `baseToken`, `_baseTokenStaking`, and `targetStaking` will be converted to `stablecoinUSDAddr` to determine the total value of the vault. ```solidity=157 function getVaultValuesInUSD() public override view returns (uint vaultValue){ uint exRateTargetBase = xWinPriceMaster.getPrice(address(baseToken), stablecoinUSDAddr); uint baseBal = _convertTo18(baseToken.balanceOf(address(this)), address(baseToken)); //ada uint baseBalUSD = exRateTargetBase * baseBal / getDecimals(address(baseToken)); //ada in USD ... } ``` Specifically, `baseBalUSD` is calculated using the formula `baseBalUSD = exRateTargetBase * baseToken.balanceOf(address(this)) / getDecimals(address(baseToken))`. However, on line 160, the code converts `baseToken.balanceOf(address(this))` to decimals of 18 prior to computing `baseBalUSD`. This conversion can potentially magnify the `baseToken` balance if the token's decimal precision is less than 18.

LowPotential Incorrect `msg.sender` References in `pFixed

In the `xWinStrategyWithFee` contract, `performanceWithdraw` uses `msg.sender` for calculating waived fees and fetching user favor. However, when a withdrawal operation is initiated through the `xWinSwap` contract, `msg.sender` is set to the `xWinSwap` contract's address, instead of the actual user address, leading to incorrect calculations. ```solidity if (unitPrice <= watermarkUnitprice || waivedPerformanceFees[msg.sender]) { return _amtOut; } ... if (lockingAddress != address(0)) { uint256 discount = ILockedStake(lockingAddress).getFavor(msg.sender); fee = fee - ((fee * discount) / 10000); } ``` This issue could lead to incorrect fee calculations and favor retrievals, which could result in financial loss for users.

LowPotential Reward Loss Caused By Not Updating PoolFixed

The functions in the given code all will update a pool's information, like the `endPeriod` and `tokenPerBlock` fields for `xWinBuddyChef` and the `totalAllocPoint` fields for `xWinMasterChef`. If these significant fields were not updated properly, the calculation for the reward would be inaccurate. For instance, in the `set()` function, the boolean flag `_withUpdate` indicates whether to update all pools before updating the specific pool. The `massUpdatePools()` function will update all pools if the flag is set to true. However, changing the `tokenPerBlock` for a pool without updating the pool first can drastically change the rewards. That is because the accumulated rewards per share (i.e., `accCakePerShare`) for the pool has not been updated to reflect the latest state before the change. This can lead to incorrect reward calculations for the users of the pool. If a pool has ended and the `pool.lastRewardBlock` has not been updated, a large number of rewards will be given out unintentionally. Similarly, changing `totalAllocPoint` without updating the pool can also cause inaccurate rewards. ```solidity function set(uint _pid, bool _withUpdate, uint _newDuration, uint _tokenPerBlock) public onlyOwner { if (_withUpdate) { massUpdatePools(); } poolInfo[_pid].endPeriod = _newDuration * blocksPerDay + block.number; poolInfo[_pid].tokenPerBlock = _tokenPerBlock; } ``` ```solidity function set(uint256 _pid, uint256 _allocPoint, bool _withUpdate, uint256 _newDuration) external onlyOwner { if (_withUpdate) { massUpdatePools(); } uint256 prevAllocPoint = poolInfo[_pid].allocPoint; poolInfo[_pid].allocPoint = _allocPoint; poolInfo[_pid].endPeriod = _newDuration * blocksPerDay + block.number; if (prevAllocPoint != _allocPoint) { totalAllocPoint = totalAllocPoint - prevAllocPoint + _allocPoint; // updateStakingPool(); } } ```

LowPotential Sandwich AttacksFixed

The `_swapTokenToToken()` function utilize the `getAmountsOut()` function of the PancakeSwap router contract and then uses this amount to call the `swapExactTokensForTokens()` and an incorrect `swapExactTokensForETHSupportingFeeOnTransferTokens()` function to execute the swap. ```solidity function _swapTokenToToken(uint amountIn, address[] memory path, address router, address destination, uint256 slippage) internal returns (uint){ ... uint256[] memory amounts = swapRouter.getAmountsOut(amountIn, path); uint256 amountOut = amounts[amounts.length-1]; if(useSupportingFeeOnTransferTokens[path[0]]){ swapRouter.swapExactTokensForETHSupportingFeeOnTransferTokens( amountIn, amountIn + (amountIn * slippage / 10000), path, destination, block.timestamp ); return amounts[amounts.length-1]; }else{ uint[] memory swapAmounts = swapRouter.swapExactTokensForTokens( amountIn, amountOut - (amountOut * slippage / 10000), path, destination, block.timestamp ); return swapAmounts[swapAmounts.length - 1]; } } ``` However, the output amount of the `getAmountsOut()` function is the same as the result of `swapExactTokensForTokens()` during the execution of this `_swapTokenToToken()` function, which means the function is vulnerable to sandwich attacks. An attacker could manipulate the expected output amount returned by the `getAmountsOut()` function by sending a large transaction that buys a small amount of the output token right before the user's transaction, and then selling that same small amount of output token right after the user's transaction. Also, there is another error passing `amountIn` as the `minAmountOut` parameter to call `swapExactTokensForETHSupportingFeeOnTransferTokens()`.

LowRepeated Function CallsFixed

The `masterChef.deposit(xwinpid, 0)` and `harvestLockBonus()` functions called within the `_doWithdraw()` function do not need to be called again, as both functions are already called within the `withdraw()` function's `_harvest()` function. ```solidity function _harvest() internal { masterChef.deposit(xwinpid, 0); uint256 bal = available(); uint256 currentPerformanceFee = bal * performanceFee / 10000; token.safeTransfer(treasury, currentPerformanceFee); uint256 currentCallFee = bal * callFee / 10000; token.safeTransfer(msg.sender, currentCallFee); harvestLockBonus(); _earn(); lastHarvestedTime = block.timestamp; } function _earn() internal { uint256 bal = available(); if (bal > 0) { masterChef.deposit(xwinpid, bal); } } function withdraw() nonReentrant external{ ... _harvest(); delete userInfo[msg.sender]; uint256 withdrawAmount = _doWithdraw(user); ... } function _doWithdraw(UserInfo memory user) internal returns (uint256) { ... masterChef.deposit(xwinpid, 0); // harvest pool 0 harvestLockBonus(); ... } ```

LowThe standard for the decimal of `getVaultValues()Fixed

According to the design of all strategies, the `getVaultValues()` and `getVaultValuesInUSD()` is used to estimate the value of the strategy tokens. However, the decimals or underlying tokens for these functions are unclear, which could lead to potential inconsistencies. - xWinDollarCostAverage.sol - `getVaultValues()`: `18` - The same result as `getVaultValuesInUSD()`. - `getVaultValuesInUSD()`: `18` - According to the `uint usdtBal = _convertTo18(stablecoin.balanceOf(address(this)), address(stablecoin))`, it assumes the `stablecoin` has the same price as USD, which could be an issue when the `stablecoin` unpegged from the dollar. - xWinERC20Alpha.sol - `getVaultValues()`: The decimals of `baseToken` - `getVaultValuesInUSD()`: The decimals of `stablecoinUSDAddr` - It assumes the decimals of `stablecoinUSDAddr` is `18`, which could be inconsistent with other strategies. - xWinIRT.sol - `getVaultValues()`: `18` - `_convertTo18(getVaultValuesInUSD(), stablecoinUSDAddr)`: It assumes `stablecoin` has the same price as `stablecoinUSDAddr`. - `getVaultValuesInUSD()`: The decimals of `stablecoin` - The decimals of `stablecoin` may be inconsistent with other strategies. - xWinSingleAsset - `getVaultValues()`: `18` - `_convertTo18(_getVaultValues(), address(targetToken))`: The decimals of the return value of the `_getVaultValues()` function are the same as the `targetToken`, but it is forcibly converted to `18`. - `getVaultValuesInUSD()`: `18` - `_convertTo18(_getVaultValuesInUSD(), stablecoinUSDAddr)`: The decimals of the return value of the `_getVaultValuesInUSD()` function are the same as the `stablecoinUSDAddr`, but it is forcibly converted to `18`. - xWinBBMA.sol - `getVaultValues()`: `18` - The same result as `getVaultValuesInUSD()`. - `getVaultValuesInUSD()`: `18` - `_convertTo18(_getVaultValuesInUSD(), stablecoinUSDAddr)`: The decimals of the return value of the `_getVaultValuesInUSD()` function are the same as the `stablecoin`, instead of `stablecoinUSDAddr`. - fundV2.sol - `getVaultValues()`: `18` - `_convertTo18(_getVaultValues(), baseCcy)`: The decimals of the return value of the `_getVaultValues()` function are the same as the `baseCcy`, but it is forcibly converted to `18`. - `getVaultValuesInUSD()`: `18` - `_convertTo18(_getVaultValuesInUSD(), stablecoinUSDAddr)`: The decimals of the return value of the `_getVaultValuesInUSD()` function are the same as the `stablecoinUSDAddr`, but it is forcibly converted to `18`.

LowThe Time to Update `stopLossPrice` and `takeProfiFixed

The `_setStopLossBuy()` and `_setTakeProfitBuy()` functions are only called inside the `_systemTrade` function when `_tradeType` is 0 (BUY), which means the `stopLossPrice` and `takeProfitPrice` variables are zero after initialization. If `performUpkeep()` can be triggered before any BUY trade has occurred, it might lead to unexpected behavior, as the take profit prices have not been set properly, which could result in the swap after the first deposit operation at a bad price. Moreover, when `_tradeType` is 1 (SELL), the `stopLossPrice` and `takeProfitPrice` variables will not be updated. This can indeed lead to the `performUpkeep()` function being triggered constantly if the price conditions are still met after a SELL trade.

Show 7 more

LowUncheck Return Value Of `xWinSwap.swapTokenToTokeFixed

The return value of `swapOutput` from the `_moveNonIndex()` function is always 0. ```solidity=372 function _moveNonIndex( address _token ) internal returns (uint256 balanceToken, uint256 swapOutput) { balanceToken = getBalance(_token); xWinSwap.swapTokenToToken(balanceToken, _token, baseCcy); return (balanceToken, 0); } ```

LowUnchecked ERC-20 `transfer()`/`transferFrom()` CaFixed

The return values of the `transfer()` and `transferFrom()` calls in the smart contract are not checked. Some ERC-20 tokens' transfer functions return no values, while others return a bool value, they should be handled with care. If a function returns `false` instead of reverting upon failure, an unchecked failed transfer could be mistakenly considered successful in the contract. ```solidity=90 _rewardToken.transferFrom(msg.sender, address(this), _totalBuddyTokens); ``` --- ```solidity=108 pool.rewardToken.transferFrom(msg.sender, address(this), _amountToAdd); ``` --- ```solidity=64 IERC20Upgradeable(tokens[i]).transfer(managers[j].managerAddress, (tokenBalance*managers[j].ratio[periodToUse]) / 10000); ``` --- ```solidity=118 IERC20Upgradeable(baseToken).transfer(strategyManager, fee); ```

LowUninitialized `lockpid` And Potential Conflict BeFixed

The `masterChefDeposit()` is used to set the `lockpid` variable, which is the ID of the pool in the MasterChef contract where the tokens will be deposited for staking. However, if the `lockpid` variable is only assigned within the `masterChefDeposit()` function and not initialized in the `initialize()` function, it may lead to unintended behavior or conflicts. If the `lockpid` variable is uninitialized, it will default to zero. This may cause an issue if the MasterChef contract already has a pool with a `pid` of 0. Similarly, it is possible that the `lockpid` could conflict with `xwinpid` if they both refer to the same pool. ```solidity function masterChefDeposit(uint256 _lockpid) external onlyOwner { // mint 1 token and put stake into locked masterChef pool _mint(address(this), 1 ether); IERC20Upgradeable(address(this)).safeApprove(address(masterChef), 1 ether); lockpid = _lockpid; masterChef.deposit(_lockpid, 1 ether); } ```

LowUninitialized State VariableFixed

The `xWinPriceMaster` variable is only assigned in the setter function `updatePriceMaster()` instead of in the `initialize()` function.

LowUnnecessary Check For `rate`Fixed

In the `getPrice()` function, there is a check for `rate > 0`. Therefore, there is no need to check again in the `_getLPPrice()` function. ```solidity function getPrice(address _from, address _to) public view returns (uint rate){ ... require(rate > 0, "PriceMaster: unsupported input addresses"); return rate; ... } function _getLPPrice(address _from, address _to) internal view returns (uint rate){ ... rate = getPrice(underlying, _to); if (rate == 0) { return 0; } ... } ```

LowUnsafe Integer CastFixed

--- ```solidity=436 int256 profit = int256((shares * IxWinStrategy(fundV2Array[i]).getUnitPrice() / decimal)) - int256((shares * avgPrice / decimal)); ``` - The type conversion `int256((shares * IxWinStrategy(fundV2Array[i]).getUnitPrice() / decimal))` from type uint256 to type int256 may flip the value's sign. --- ```solidity=442 int256 profitUSD = profit * int256(rate) / int256(decimal); ``` - The type conversion `int256(rate)` from type uint256 to type int256 may flip the value's sign. --- ```solidity=76 uint256 rateTo = chainlinkDecimalsTo * chainlinkDecimalsTo / uint(priceTo); ``` - The type conversion `uint256(priceTo)` from type int256 to type uint256 may flip the value's sign. --- ```solidity=77 rate = uint(priceFrom) * rateTo * decimalPlaces / chainlinkDecimalsFrom /chainlinkDecimalsTo; ``` - The type conversion `uint256(priceFrom)` from type int256 to type uint256 may flip the value's sign. --- ```solidity=82 rate = uint(price) * decimalPlaces / chainlinkDecimals; ``` - The type conversion `uint256(price)` from type int256 to type uint256 may flip the value's sign.

LowUnused Functions and VariablesFixed

The linked state variables and functions are unused and do not seem to have any functionality in the contract.