# KillSwitch Finance - AutoCompound ###### tags: `Audit Project`, `Finished` [TOC] ## Findings Summary During the audit period, Inspex found 6 Critical, 2 High, 1 Medium, 3 Low, 2 Very Low, 4 Info-severity issues as shown in the following chart. ```vega { "$schema": "https://vega.github.io/schema/vega-lite/v5.json", "description": "Severity Information", "width": {"step": 60}, "data": { "values": [ { "Severity": "Critical", "color": "#b70229", "Count": 6 }, { "Severity": "High", "color": "#ec3d27", "Count": 2 }, { "Severity": "Medium", "color": "#fb8348", "Count": 1 }, { "Severity": "Low", "color": "#fecc69", "Count": 3 }, { "Severity": "Very Low", "color": "#ffffb7", "Count": 2 }, { "Severity": "Info", "color": "#337ab7", "Count": 4 } ] }, "mark": "bar", "encoding": { "x": { "field": "Severity", "type": "nominal", "sort": ["Critical", "High", "Medium", "Low", "Very Low", "Info"] }, "y": { "field": "Count", "type": "quantitative" }, "color": { "field": "color", "type": "nominal", "scale": null } } } ``` The following table shows all issues found in this audit activity. | ID | Issue | Category | Severity | | ------- | ------------------------------------------------------------------------------ | ------------- | -------- | | IDX-001 | Transaction Ordering Dependence | General | Critical | | IDX-002 | Token Draining Using Izlude.setMorroc() Function | Advanced | Critical | | IDX-003 | Token Draining Using Izlude.upgradeStrategy() Function | Advanced | Critical | | IDX-004 | Token Draining Using PancakeByalanLP.setIzlude() Function | Advanced | Critical | | IDX-005 | Token Draining Using PancakeByalanLP.setUnirouter() Function | Advanced | Critical | | IDX-006 | Token Draining Using PronteraCastle.setProntera() Function | Advanced | Critical | | IDX-007 | Setting of Excessively High Fees | Advanced | High | | IDX-008 | Centralized Control of State Variable | General | High | | IDX-009 | Improper Reward Calculation | Advanced | Medium | | IDX-010 | Improper Fee Setting (Performance Fee) | Advanced | Low | | IDX-011 | Improper Fee Setting (Deposit and Withdraw Fee) | Advanced | Low | | IDX-012 | Design Flaw in massUpdatePools() Function | General | Low | | IDX-013 | Dangerous Approval to External Contract | Advanced | Very Low | | IDX-014 | Insufficient Logging for Privileged Functions | Advanced | Very Low | | IDX-015 | Improper Reentrancy Attack Protection Mechanism | General | Info | | IDX-016 | Improper Token Transfer | General | Info | | IDX-017 | Improper Function Visibility | Best Practice | Info | | IDX-018 | Inexplicit Solidity Compiler Version | Best Practice | Info | Please fill in the following sheet with the remediation done for each issue: https://docs.google.com/spreadsheets/d/1JRH3GoNZKKwinSaihB0ZR8LM1KhJ0dJ5LiKHjqL4hZ4/edit?usp=sharing ## Findings Detail ### IDX-001 Transaction Ordering Dependence - **Target:** - PancakeAlbertaBNBLP - PancakeAlbertaBUSDLP - PancakeByalanLP - **Category:** General Smart Contract Vulnerability - **CWE:** CWE-362: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') - **Severity:** Critical - **Impact:** High Attackers can perform front-running attack to gain profit, resulting in users losing a portion of the expected LP token from bad-rate swapping. - **Likelihood:** High It is very easy to perform the attack. Moreover, anyone that monitors the BSC's transaction pool, can attack users with this issue. - **Status:** Open #### Description When users want to start farming to a particular pool, the `PancakeAlbertaBNBLP` contract and the `PancakeAlbertaBUSDLP` contract are responsible to make LP token by swapping the half of input token (`_paidToken`) to another token (`_otherToken`). During swapping tokens, there is a potential bad-rate swapping since `ROUTER.swapExactTokensForETH(amountIn, 0, pathPaid, address(this), block.timestamp)` takes `0` as `amountOutMin`; however, there is the `_safePriceImpactSwap()` function to check the price impact before swapping, which unfortunately could not prevent the bad-rate swapping. **PancakeAlbertaBNBLP.sol** ```solidity=1023 function add( address _paidToken, address _wantToken, address _to, uint256 _amountIn, uint256 _maxPriceImpact ) external payable override nonReentrant { IUniswapV2Pair lpToken = IUniswapV2Pair(_wantToken); address otherToken = lpToken.token0() == WBNB ? lpToken.token1() : lpToken.token0(); // Preparing BNB or farming tokens if (_paidToken != address(0) && _paidToken != otherToken) { IERC20(_paidToken).safeTransferFrom(msg.sender, address(this), _amountIn); address[] memory pathPaid = new address[](2); (pathPaid[0], pathPaid[1]) = (_paidToken, WBNB); IERC20(_paidToken).safeApprove(address(ROUTER), 0); IERC20(_paidToken).safeApprove(address(ROUTER), type(uint256).max); uint256 amountIn = IERC20(_paidToken).balanceOf(address(this)); _safePriceImpactSwap(amountIn, pathPaid, _maxPriceImpact); ROUTER.swapExactTokensForETH(amountIn, 0, pathPaid, address(this), block.timestamp); } else if (_paidToken == otherToken) { IERC20(otherToken).safeTransferFrom(msg.sender, address(this), _amountIn); } // Compute the optimal amount of BNB and otherToken to be converted. uint256 bnbBalance = address(this).balance; uint256 swapAmt; bool isReversed; { (uint256 r0, uint256 r1, ) = lpToken.getReserves(); (uint256 bnbReserve, uint256 otherReserve) = lpToken.token0() == WBNB ? (r0, r1) : (r1, r0); (swapAmt, isReversed) = optimalDeposit( bnbBalance, IERC20(otherToken).balanceOf(address(this)), bnbReserve, otherReserve ); } // Convert between BNB and farming tokens IERC20(otherToken).safeApprove(address(ROUTER), 0); IERC20(otherToken).safeApprove(address(ROUTER), type(uint256).max); address[] memory path = new address[](2); (path[0], path[1]) = isReversed ? (otherToken, WBNB) : (WBNB, otherToken); _safePriceImpactSwap(swapAmt, path, _maxPriceImpact); if (isReversed) { ROUTER.swapExactTokensForETH(swapAmt, 0, path, address(this), block.timestamp); // farming tokens to BNB } else { ROUTER.swapExactETHForTokens{value: swapAmt}(0, path, address(this), block.timestamp); // BNB to farming tokens } // Mint more LP tokens and return all LP tokens to the sender. ROUTER.addLiquidityETH{value: address(this).balance}( otherToken, IERC20(otherToken).balanceOf(address(this)), 0, 0, _to, block.timestamp ); } ``` **PancakeAlbertaBNBLP.sol** ```solidity=1127 function _safePriceImpactSwap( uint256 _amountIn, address[] memory _path, uint256 _maxPriceImpact ) private view { if (_maxPriceImpact == 0) { return; } require(_path.length >= 2, "INVALID_PATH"); for (uint256 i; i < _path.length - 1; i++) { (uint256 rIn, uint256 rOut) = PancakeLibrary.getReserves(address(FACTORY), _path[i], _path[i + 1]); uint256 q = ROUTER.quote(_amountIn, rIn, rOut); uint256 out = PancakeLibrary.getAmountOut(_amountIn, rIn, rOut); uint256 priceImpact = ((q - out) * 100000) / q; require(priceImpact <= _maxPriceImpact, "price impact too high"); _amountIn = out; } } ``` An example below demonstrates that checking price impact before swapping cannot prevent bad-rate swapping: The formula to calculate the price impact is as follows (swapping fee is ignored): ``` quote = amountIn * reserveOut / reserveIn output = amountIn * reserveOut / (reserveIn + amountIn) priceImpact = (quote - output) * 100000 / quote ``` Assuming the reserve amounts of tokens in pool before being manipulated are as follows: | reserveBNB | reserveB | | ---------- | -------- | | 50 | 50 | The contract swaps 5 $BNB to $B (`otherToken`) with `_maxPriceImpact` as 10000 (10%). ``` quote = 5 * 50 / 50 = 5 output = 5 * 50 / (50 + 5) = 4.54 priceImpact = (5 - 4.54) * 100000/ 5 = 0.092 * 100000 = 9200 (9.2%) ``` As a result, swapping 5 $BNB will get 4.54 $B. However, if this transaction is being front-run, the price will be worse. | reserveBNB | reserveB | | ---------- | -------- | | 60 | 41.67 | The contract swaps 5 $BNB to $B (`otherToken`) with `_maxPriceImpact` as 10000 (10%). ``` quote = 5 * 41.67 / 60 = 3.4725 output = 5 * 41.67 / (60 + 5) = 3.2053 priceImpact = (3.4725 - 3.2053) * 100000 / 3.4725 = 0.0769 * 100000 = 7690 (7.69%) ``` As a result, swapping 5 $BNB will get 3.2053 $B. Hence, the amount of received tokens from swapping is affected from the transaction ordering dependence. The table below represents the contracts and functions with potential bad-rate swapping: | Contract | Function | | -------------------- | -------------- | | PancakeAlbertaBNBLP | add() | | PancakeAlbertaBNBLP | remove() | | PancakeAlbertaBUSDLP | add() | | PancakeAlbertaBUSDLP | remove() | | PancakeByalanLP | chargeFees() | | PancakeByalanLP | addLiquidity() | #### Remediation Inspex suggests that the contracts should take `amountOutMin` as a consideration when swapping as in line 1029 and line 1045, for example: **PancakeAlbertaBNBLP.sol** ```solidity=1023 function add( address _paidToken, address _wantToken, address _to, uint256 _amountIn, uint256 _maxPriceImpact uint256 _amountOutMin ) external payable override nonReentrant { IUniswapV2Pair lpToken = IUniswapV2Pair(_wantToken); address otherToken = lpToken.token0() == WBNB ? lpToken.token1() : lpToken.token0(); // Preparing BNB or farming tokens if (_paidToken != address(0) && _paidToken != otherToken) { IERC20(_paidToken).safeTransferFrom(msg.sender, address(this), _amountIn); address[] memory pathPaid = new address[](2); (pathPaid[0], pathPaid[1]) = (_paidToken, WBNB); IERC20(_paidToken).safeApprove(address(ROUTER), 0); IERC20(_paidToken).safeApprove(address(ROUTER), type(uint256).max); uint256 amountIn = IERC20(_paidToken).balanceOf(address(this)); _safePriceImpactSwap(amountIn, pathPaid, _maxPriceImpact); ROUTER.swapExactTokensForETH(amountIn, _amountOutMin, pathPaid, address(this), block.timestamp); } else if (_paidToken == otherToken) { IERC20(otherToken).safeTransferFrom(msg.sender, address(this), _amountIn); } ... } ``` ### IDX-002 Token Draining Using Izlude.setMorroc() Function - **Target:** Izlude - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-284: Improper Access Control - **Severity:** Critical - **Impact:** High The owner can drain the tokens staked by the users to the owner's wallet. - **Likelihood:** High There is no restriction to prevent the owner from performing this attack. - **Status:** Open #### Description In the `Izlude` contract, the owner of the contract can use the `setMorroc()` function to set the `morroc` state variable. **Izlude.sol** ```solidity=728 function setMorroc(address _morroc) external onlyOwner { morroc = _morroc; } ``` The address in the `morroc` variable can call the functions with the `onlyMorroc` modifier. **Izlude.sol** ```solidity=723 modifier onlyMorroc() { require(_msgSender() == morroc, "!morroc"); _; } ``` The `withdrawFromMorroc()` function is one of the functions with the `onlyMorroc` modifier. It can be used to withdraw the user's share using the `_withdraw()` function. **Izlude.sol** ```solidity=889 function withdrawFromMorroc(uint256 _jellopy, address _user) external onlyMorroc returns (uint256) { return _withdraw(_jellopy, _user); } ``` The `_withdraw()` function deducts the user's share in line 864, withdraws the tokens from the farm using `byalan.withdraw()` function in line 869, and then transfers the tokens withdrawn to the function caller in line 883. **Izlude.sol** ```solidity=862 function _withdraw(uint256 _jellopy, address _user) private nonReentrant returns (uint256) { uint256 r = (balance() * _jellopy) / totalSupply(); _burn(_user, _jellopy); uint256 b = want().balanceOf(address(this)); if (b < r) { uint256 amount = r - b; byalan.withdraw(amount); uint256 _after = want().balanceOf(address(this)); uint256 diff = _after - b; if (diff < amount) { r = b + diff; } } uint256 fee = calculateWithdrawFee(r, _user); if (fee > 0) { r -= fee; want().safeTransfer(address(feeKafra), fee); feeKafra.distributeWithdrawFee(want(), _user); } want().safeTransfer(_msgSender(), r); return r; } ``` This means that if the owner uses the `setMorroc()` function to set the `morroc` variable as the owner's wallet, the owner can use that account to execute the `withdrawFromMorroc()` function to transfer the token staked by each user to the owner's wallet. #### Remediation Inspex suggests removing the `setMorroc()` function from the `Izlude` contract, and set the address from the constructor. However, if modifications are needed, it is recommended to limit the use of this function via the following options: - Implementing a community-run governance to control the use of this function - Using a Timelock contract to delay the changes for a sufficient amount of time ### IDX-003 Token Draining Using Izlude.upgradeStrategy() Function - **Target:** Izlude - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-284: Improper Access Control - **Severity:** Critical - **Impact:** High The owner can drain the tokens staked by the users to the owner's wallet. - **Likelihood:** High There is no restriction to prevent the owner from performing this attack. - **Status:** Open #### Description In the `Izlude` contract, the `proposeStrategy()` and `upgradeStrategy()` functions can be used by the contract owner to change the address of `byalan` variable in the contract. The `proposeStrategy()` needs to be called first to set the `byalanCandidate` variable. **Izlude.sol** ```solidity=897 function proposeStrategy(address _implementation) external onlyOwner { require(address(this) == IByalan(_implementation).izlude(), "proposal invalid"); byalanCandidate = ByalanCandidate({implementation: _implementation, proposedTime: block.timestamp}); emit NewStrategyCandidate(_implementation); } ``` After that, the `upgradeStrategy()` function can be executed to set the `byalan` using the value of `byalanCandidate` set in the previous step. There is an `approvalDelay` variable to prevent this function from being executed instantly after `proposeStrategy()` function. **Izlude.sol** ```solidity=909 function upgradeStrategy() external onlyOwner nonReentrant { require(byalanCandidate.implementation != address(0), "There is no candidate"); require(byalanCandidate.proposedTime + approvalDelay < block.timestamp, "Delay has not passed"); emit UpgradeStrategy(byalanCandidate.implementation); byalan.retireStrategy(); byalan = IByalan(byalanCandidate.implementation); byalanCandidate.implementation = address(0); byalanCandidate.proposedTime = 5000000000; earn(); } ``` However, there is no minimum `approvalDelay`, potentially allowing an instant changing of `byalan` variable. **Izlude.sol** ```solidity=718 constructor(IByalan _byalan, uint256 _approvalDelay) { byalan = _byalan; approvalDelay = _approvalDelay; } ``` By changing `byalan` to the address of a malicious contract, the users' funds will be transferred to the new `byalan` contract when the `earn()` function is called. **Izlude.sol** ```solidity=853 function earn() public { uint256 _bal = available(); want().safeTransfer(address(byalan), _bal); byalan.deposit(); } ``` #### Remediation Inspex suggests removing the `proposeStrategy()` and `upgradeStrategy()` functions from the `Izlude` contract. However, if modifications are needed, it is recommended to limit the use of this function by setting a minimum `approvalDelay` in the contract to delay the changing of `byalan` for a sufficient amount of time, for example: **Izlude.sol** ```solidity=718 uint256 MIN_DELAY = 3 days; constructor(IByalan _byalan, uint256 _approvalDelay) { require(_approvalDelay >= MIN_DELAY, "Approval delay is too short.") byalan = _byalan; approvalDelay = _approvalDelay; } ``` As another option, if modifications are needed, it is recommended to limit the use of this function via the following options: - Implementing a community-run governance to control the use of this function - Using a Timelock contract to delay the changes for a sufficient amount of time ### IDX-004 Token Draining Using PancakeByalanLP.setIzlude() Function - **Target:** PancakeByalanLP - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-284: Improper Access Control - **Severity:** Critical - **Impact:** High The owner can drain the tokens staked by the users to the owner's wallet. - **Likelihood:** High There is no restriction to prevent the owner from performing this attack. - **Status:** Open #### Description In the `PancakeByalanLP` contract, the owner can set `izlude` variable freely using the `setIzlude()` function. **PancakeByalanBP.sol** ```solidity=1128 function setIzlude(address _izlude) external onlyOwner { izlude = _izlude; } ``` This allows the address to execute any function with `onlyIzlude` privilege. **PancakeByalanBP.sol** ```solidity=1110 modifier onlyIzlude() { require(msg.sender == izlude, "!izlude"); _; } ``` The `retireStatregy()` function can be used for withdrawing all LP tokens in `MASTERCHEF`. The LP tokens will be transferred to the `izlude` contract. **PancakeByalanBP.sol** ```solidity=1396 function retireStrategy() external override onlyIzlude { IMasterChef(MASTERCHEF).emergencyWithdraw(pid); uint256 wantBal = IERC20(want).balanceOf(address(this)); IERC20(want).transfer(izlude, wantBal); } ``` This means the owner can set the owner's address as `izlude` and execute the `retireStrategy()` function to drain all LP tokens. #### Remediation Inspex suggests removing the `setIzlude()` function from the `PancakeByalanLP` contract. However, if modifications are needed, it is recommended to limit the use of this function via the following options: - Implementing a community-run governance to control the use of this function - Using a Timelock contract to delay the changes for a sufficient amount of time ### IDX-005 Token Draining Using PancakeByalanLP.setUnirouter() Function - **Target:** ByalanIsland - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-284: Improper Access Control - **Severity:** Critical - **Impact:** High The owner can steal tokens every time tokens are swapped inside the `PancakeByalanLP` contract. - **Likelihood:** High There is no restriction to prevent the owner from performing this attack. - **Status:** Open #### Description The `PancakeByalanLP` contract inherits the `ByalanIsland` contract and uses the contract in `unirouter` variable to swap $CAKE rewards to other tokens in line 1341 and 1351. **PancakeByalanLP.sol** ```solidity=1337 function addLiquidity() internal { uint256 cakeHalf = IERC20(CAKE).balanceOf(address(this)) / 2; if (lpToken0 != CAKE) { IUniswapV2Router02(unirouter).swapExactTokensForTokens( cakeHalf, 0, cakeToLp0Route, address(this), block.timestamp ); } if (lpToken1 != CAKE) { IUniswapV2Router02(unirouter).swapExactTokensForTokens( cakeHalf, 0, cakeToLp1Route, address(this), block.timestamp ); } IUniswapV2Router02(unirouter).addLiquidity( lpToken0, lpToken1, IERC20(lpToken0).balanceOf(address(this)), IERC20(lpToken1).balanceOf(address(this)), 0, 0, address(this), block.timestamp ); } ``` The owner can set the `unirouter` variable using the `setUnirouter()` function, allowing the owner to set a malicious contract as a router and steal the $CAKE rewards. **PancakeByalanLP.sol** ```solidity=1124 function setUnirouter(address _unirouter) external onlyOwner { unirouter = _unirouter; } ``` #### Remediation Inspex suggests removing the `setUnirouter()` function from the `PancakeByalanLP` contract (`ByalanIsland` abstract contract). However, if modifications are needed, it is recommended to limit the use of this function via the following options: - Implementing a community-run governance to control the use of this function - Using a Timelock contract to delay the changes for a sufficient amount of time ### IDX-006 Token Draining Using PronteraCastle.setProntera() Function - **Target:** PronteraCastle - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-284: Improper Access Control - **Severity:** Critical - **Impact:** High The owner can steal all tokens allocated as the staking rewards. - **Likelihood:** High There is no restriction to prevent the owner from performing this attack. - **Status:** Open #### Description The `PronteraCastle` contract is used for storing funds. It allows the owner of the contract to execute all of withdrawal operations, except `stakingWithdraw()` function which can be called by the address inside the `prontera` variable only. **Morroc.sol** ```solidity=1543 function stakingWithdraw(address _to, uint256 _amount) external { require(msg.sender == prontera, "!prontera"); _safeTransfer(Category.STAKING, _to, _amount); } ``` However, with the `setProntera()` function, the owner can set the `prontera` variable to be any address without restriction. **Morroc.sol** ```solidity=1455 function setProntera(address _prontera) external onlyOwner { prontera = _prontera; } ``` Therefore, by setting `prontera` to the owner's address, the owner can withdraw all of the funds in the staking category freely. #### Remediation Inspex suggests removing the `setProntera()` function from the `PronteraCastle` contract and set the address in the constructor. However, if modifications are needed, it is recommended to limit the use of this function via the following options: - Implementing a community-run governance to control the use of this function - Using a Timelock contract to delay the changes for a sufficient amount of time ### IDX-007 Setting of Excessively High Fees - **Target:** Izlude - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-284: Improper Access Control - **Severity:** High - **Impact:** High The owner can steal the tokens deposited or withdrawn by the users by setting the fees to be 100%. - **Likelihood:** Medium There is no restriction to prevent the owner from performing this attack; however, this attack requires the users to deposit or withdraw for the attacker to steal the tokens. - **Status:** Open #### Description In the `Izlude` contract, the owner of the contract can use the `setFeeKafra()` function to set the `feeKafra` state variable. **Izlude.sol** ```solidity=732 function setFeeKafra(IFeeKafra _feeKafra) external onlyOwner { feeKafra = _feeKafra; } ``` The `feeKafra` variable stores the address of the `FeeKafra` contract responsible for calculating and distributing the fees. **Izlude.sol** ```solidity=796 function calculateDepositFee(uint256 _amount, address _user) public view returns (uint256) { if (address(feeKafra) == address(0)) { return 0; } return feeKafra.calculateDepositFee(_amount, _user); } function calculateWithdrawFee(uint256 _amount, address _user) public view returns (uint256) { if (address(feeKafra) == address(0)) { return 0; } return feeKafra.calculateWithdrawFee(_amount, _user); } ``` The fee is calculated every time the `_deposit()` or `_withdraw()` functions are called. In the `_deposit()` function, the deposit fee is calculated in line 818, which is then transferred to the `feeKafra` address in line 820. **Izlude.sol** ```solidity=810 function _deposit(uint256 _amount, address _user) private nonReentrant returns (uint256) { byalan.beforeDeposit(); uint256 _pool = balance(); want().safeTransferFrom(_msgSender(), address(this), _amount); uint256 _after = balance(); _amount = _after - _pool; uint256 fee = calculateDepositFee(_amount, _user); if (fee > 0) { want().safeTransfer(address(feeKafra), fee); feeKafra.distributeDepositFee(want(), _user); } earn(); _after = balance(); _amount = _after - _pool; // Additional check for deflationary tokens require( address(allocKafra) == address(0) || allocKafra.canAllocate(_amount, byalan.balanceOf(), byalan.balanceOfMasterChef(), _user), "capacity limit reached" ); uint256 jellopy = 0; if (totalSupply() == 0) { jellopy = _amount; } else { jellopy = (_amount * totalSupply()) / _pool; } _mint(_user, jellopy); return jellopy; } ``` In the `_withdraw()` function, the withdrawal fee is calculated in line 877, which is then transferred to the `feeKafra` address in line 880. **Izlude.sol** ```solidity=862 function _withdraw(uint256 _jellopy, address _user) private nonReentrant returns (uint256) { uint256 r = (balance() * _jellopy) / totalSupply(); _burn(_user, _jellopy); uint256 b = want().balanceOf(address(this)); if (b < r) { uint256 amount = r - b; byalan.withdraw(amount); uint256 _after = want().balanceOf(address(this)); uint256 diff = _after - b; if (diff < amount) { r = b + diff; } } uint256 fee = calculateWithdrawFee(r, _user); if (fee > 0) { r -= fee; want().safeTransfer(address(feeKafra), fee); feeKafra.distributeWithdrawFee(want(), _user); } want().safeTransfer(_msgSender(), r); return r; } ``` The deposit and withdrawal fees are capped in the `FeeKafra` contract, preventing the fees from being set to an excessively high value. **FeeKafra.sol** ```solidity=569 function setDepositFee(uint256 _fee) external onlyOwner { require(_fee <= MAX_DEPOSIT_FEE, "!cap"); depositFee = _fee; } function setWithdrawFee(uint256 _fee) external onlyOwner { require(_fee <= MAX_WITHDRAW_FEE, "!cap"); withdrawFee = _fee; } ``` However, since the owner can set the value of `feeKafra` variable, the owner can set the `feeKafra` to a malicious contract with 100% fees to steal the funds of the users on each deposit or withdraw. #### Remediation Inspex suggests removing the `setFeeKafra()` function from the `Izlude` contract, and set the address from the constructor. However, if modifications are needed, it is recommended to limit the use of this function via the following options: - Implementing a community-run governance to control the use of this function - Using a Timelock contract to delay the changes for a sufficient amount of time ### IDX-008 Centralized Control of State Variable - **Target:** - AllocKafra - FeeKafra - Izlude - PronteraCastle - PronteraGuard - Prontera - PancakeAlbertaBNBLP - PancakeAlbertaBUSDLP - ByalandIsland - Sailor - GasThrottler - PancakeByalanBP - **Category:** General Smart Contract Vulnerability - **CWE:** CWE-710: Improper Adherence to Coding Standard - **Severity:** High - **Impact:** Medium The controlling authorities can change the critical state variables to gain additional profit. Thus, it is unfair to the other users. - **Likelihood:** High There is nothing to restrict the changes from being done; however, the changes are limited by fixed values in the smart contracts. - **Status:** Open #### Description Critical state variables can be updated any time by the controlling authorities. Changes in these variables can cause impacts to the users, so the users should accept or be notified before these changes are effective. However, there is currently no constraint to prevent the authorities from modifying these variables without notifying the users. The controllable privileged state update functions are as follows: | File | Contract | Function | Modifier | | --------------------------------- | -------------------- | --------------------------- | ----------- | | AllocKafra.sol (L:148) | AllocKafra | setLimitAllocation() | onlyOwner | | FeeKafra.sol (L:569) | FeeKafra | setDepositFee() | onlyOwner | | FeeKafra.sol (L:575) | FeeKafra | setWithdrawFee() | onlyOwner | | FeeKafra.sol (L:581) | FeeKafra | setTreasuryFeeDeposit() | onlyOwner | | FeeKafra.sol (L:585) | FeeKafra | setKSWFeeDeposit() | onlyOwner | | FeeKafra.sol (L:589) | FeeKafra | setTreasuryFeeWithdraw() | onlyOwner | | FeeKafra.sol (L:593) | FeeKafra | setKSWFeeWithdraw() | onlyOwner | | FeeKafra.sol (L:597) | FeeKafra | setKSWFeeRecipient() | onlyOwner | | FeeKafra.sol (L:601) | FeeKafra | setTreasuryFeeRecipient() | onlyOwner | | Izlude.sol (L:728) | Izlude | setMorroc() | onlyOwner | | Izlude.sol (L:732) | Izlude | setFeeKafra() | onlyOwner | | Izlude.sol (L:736) | Izlude | setAllocKafra() | onlyOwner | | Izlude.sol (L:897) | Izlude | proposeStrategy() | onlyOwner | | Izlude.sol (L:909) | Izlude | upgradeStrategy() | onlyOwner | | Izlude.sol (L:928) | Izlude | inCaseTokensGetStuck() | onlyOwner | | Morroc.sol (L:1455) | PronteraCastle | setProntera() | onlyOwner | | Morroc.sol (L:1503) | PronteraCastle | founderWithdraw() | onlyOwner | | Morroc.sol (L:1507) | PronteraCastle | teamWithdraw() | onlyOwner | | Morroc.sol (L:1511) | PronteraCastle | reserveWithdraw() | onlyOwner | | Morroc.sol (L:1515) | PronteraCastle | marketingWithdraw() | onlyOwner | | Morroc.sol (L:1519) | PronteraCastle | advisorWithdraw() | onlyOwner | | Morroc.sol (L:1523) | PronteraCastle | seedFundWithdraw() | onlyOwner | | Morroc.sol (L:1527) | PronteraCastle | privateSaleWithdraw() | onlyOwner | | Morroc.sol (L:1531) | PronteraCastle | publicSaleWithdraw() | onlyOwner | | Morroc.sol (L:1535) | PronteraCastle | exchangeLiquidityWithdraw() | onlyOwner | | Morroc.sol (L:1539) | PronteraCastle | airDropWithdraw() | onlyOwner | | Morroc.sol (L:1594) | PronteraGuard | addGuard() | onlyOwner | | Morroc.sol (L:1601) | PronteraGuard | removeGuard() | onlyOwner | | Morroc.sol (L:1675) | Prontera | add() | onlyOwner | | Morroc.sol (L:1703) | Prontera | set() | onlyOwner | | Morroc.sol (L:1787) | Prontera | setKSWPerBlock() | onlyOwner | | Morroc.sol (L:1793) | Prontera | setStartBlock() | onlyOwner | | PancakeAlbertaBNBLP.sol (L:1148) | PancakeAlbertaBNBLP | recover() | onlyOwner | | PancakeAlbertaBNBLP.sol (L:1152) | PancakeAlbertaBNBLP | recoverToken() | onlyOwner | | PancakeAlbertaBUSDLP.sol (L:1173) | PancakeAlbertaBUSDLP | recover() | onlyOwner | | PancakeAlbertaBUSDLP.sol (L:1177) | PancakeAlbertaBUSDLP | recoverToken() | onlyOwner | | PancakeByalanLP.sol (L:1120) | ByalanIsland | setHydra() | onlyManager | | PancakeByalanLP.sol (L:1124) | ByalanIsland | setUnirouter() | onlyOwner | | PancakeByalanLP.sol (L:1128) | ByalanIsland | setIzlude() | onlyOwner | | PancakeByalanLP.sol (L:1132) | ByalanIsland | setTreasuryFeeRecipient() | onlyOwner | | PancakeByalanLP.sol (L:1136) | ByalanIsland | setKswFeeRecipient() | onlyOwner | | PancakeByalanLP.sol (L:1157) | Sailor | setTotalFee() | onlyManager | | PancakeByalanLP.sol (L:1163) | Sailor | setCallFee() | onlyManager | | PancakeByalanLP.sol (L:1167) | Sailor | setTreasuryFee() | onlyManager | | PancakeByalanLP.sol (L:1171) | Sailor | setKSWFee() | onlyManager | | PancakeByalanLP.sol (L:1202) | GasThrottler | setGasPrice() | onlyOwner | | PancakeByalanLP.sol (L:1171) | PancakeByalanLP | setGasPrice() | onlyOwner | | Ownable | Ownable | renounceOwnership() | onlyOwner | | Ownable | Ownable | transferOwnership() | onlyOwner | Please note that the `Ownable` contract is inherited from the `OpenZeppelin` library. #### Remediation In the ideal case, the critical state variables should not be modifiable to keep the integrity of the smart contract. However, if modifications are needed, Inspex suggests limiting the use of these functions via the following options: - Implementing a community-run governance to control the use of these functions - Using a Timelock contract to delay the changes for a reasonable amount of time For the `Sailor` contract, modifier of `setTotalFee()`, `setCallFee()`, `setTreasuryFee()`, and `setKSWFee()` should be changed from `onlyManager` to `onlyOwner` before using a timelock because the `onlyManager` is used by necessary functions in case of emergency in `PancakeByalanLP` contract such as `pause()` and `unpause()` that should be used immediately without waiting. ### IDX-009 Improper Reward Calculation - **Target:** Prontera - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-840: Business Logic Errors - **Severity:** Medium - **Impact:** Medium The $KSW reward miscalculation can lead to unfair $KSW token distribution. - **Likelihood:** Medium This issue happens whenever the `totalAllocPoint` is modified and the `_withUpdate` parameter is set to false. - **Status:** Open #### Description The `totalAllocPoint` variable is used to determine the portion that each pool would get from the total rewards minted, so it is one of the main factors used in the rewards calculation. Therefore, whenever the `totalAllocPoint` variable is modified without updating the pending rewards first, the reward of each pool will be incorrectly calculated. In the `add()` and `set()` functions shown below, if `_withUpdate` is set to `false`, the `totalAllocPoint` variable will be modified without updating the rewards. **Morroc.sol** ```solidity=1675 function add( uint256 _allocPoint, address _izlude, bool _withUpdate ) external onlyOwner { if (poolInfo.length > 0) { uint256 pid = izludeToPool[_izlude]; PoolInfo storage pool = poolInfo[pid]; require(address(pool.izlude) != _izlude, "duplicated"); } if (_withUpdate) { massUpdatePools(); } uint256 lastRewardBlock = block.number > startBlock ? block.number : startBlock; totalAllocPoint += _allocPoint; poolInfo.push( PoolInfo({ izlude: Izlude(_izlude), allocPoint: _allocPoint, lastRewardBlock: lastRewardBlock, accKSWPerShare: 0 }) ); izludeToPool[_izlude] = poolInfo.length - 1; } ``` **Morroc.sol** ```solidity=1703 function set( uint256 _pid, uint256 _allocPoint, bool _withUpdate ) external onlyOwner { if (_withUpdate) { massUpdatePools(); } totalAllocPoint -= poolInfo[_pid].allocPoint; totalAllocPoint += _allocPoint; poolInfo[_pid].allocPoint = _allocPoint; } ``` **For example:** Assuming that on block 19999999, `kswPerBlock` is 3 $KSW per block, `totalAllocPoint` is 6700, and `allocPoint` of pool id 0 is 300. | Block | Action | | -------- | ------------------------------ | | 19999999 | All pools’ rewards are updated | | 20099999 | A new pool is added using the add() function, causing the `totalAllocPoint` to be changed from 6700 to 8000 | | 20199999 | The pools’ rewards are updated once again. | From current logic, the total rewards allocated to the pool id 0 during block 19999999 to 20199999 is equal to 22,500 $KSW calculated using the following equation: | Block | Total Block Reward | Total Allocation Point | Total $KSW per block for pool 0 (kswPerBlock*pool0allocPoint/totalAllocPoint) | Total pool 0 $KSW Reward | | ------------------- | ------------------ | ---------------------- | ----------------------------------------------------------------------------- | ------------------------ | | 19999999 - 20199999 | 20000000 | 8,000 | 0.1125 $KSW per block | 22,500 $KSW | However, the rewards should be calculated by accounting for the original `totalAllocPoint` value during the period when it is not yet updated as follows: | Block | Total Block Reward | Total Allocation Point | Total $KSW per block for pool 0 (kswPerBlock*pool0allocPoint/totalAllocPoint) | Total pool 0 $KSW Reward | | ------------------- | ------------------ | ---------------------- | ----------------------------------------------------------------------------- | ------------------------ | | 19999999 - 20099999 | 10000000 | 6,700 | 0.1343 $KSW per block | 13,432.83 $KSW | | 20099999 - 20199999 | 10000000 | 8,000 | 0.1125 $KSW per block | 11,250.00 $KSW | The correct total $KSW rewards is 24,682.83 $KSW, which is different from the miscalculated reward by 2,182.83 $KSW. #### Remediation Inspex suggests removing `_withUpdate` variable in the `add()` and `set()` functions and always calling the `massUpdatePools()` function before updating `totalAllocPoint` variable as shown in the following example: **Morroc.sol** ```solidity=1675 function add( uint256 _allocPoint, address _izlude, ) external onlyOwner { if (poolInfo.length > 0) { uint256 pid = izludeToPool[_izlude]; PoolInfo storage pool = poolInfo[pid]; require(address(pool.izlude) != _izlude, "duplicated"); } massUpdatePools(); uint256 lastRewardBlock = block.number > startBlock ? block.number : startBlock; totalAllocPoint += _allocPoint; poolInfo.push( PoolInfo({ izlude: Izlude(_izlude), allocPoint: _allocPoint, lastRewardBlock: lastRewardBlock, accKSWPerShare: 0 }) ); izludeToPool[_izlude] = poolInfo.length - 1; } ``` **Morroc.sol** ```solidity=1703 function set( uint256 _pid, uint256 _allocPoint, ) external onlyOwner { massUpdatePools(); totalAllocPoint -= poolInfo[_pid].allocPoint; totalAllocPoint += _allocPoint; poolInfo[_pid].allocPoint = _allocPoint; } ``` ### IDX-010 Improper Fee Setting (Performance Fee) - **Target:** - Sailor - PancakeByalanLP - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-755: Improper Handling of Exceptional Conditions - **Severity:** Low - **Impact:** Medium Fees can be incorrectly set, causing either the `harvest()` function of `PancakeByalanLP` contract to be unusable, or some $BNB to be stuck in the contract. - **Likelihood:** Low It is unlikely that the owner will set the fees incorrectly and leave them unfixed. - **Status:** Open #### Description In the `PancakeByalanLP` contract, the `harvest()` function can be used to collect and auto-compound the rewards into the pool. Some fees are deducted as the performance fee from a part of the reward collected with the `chargeFees()` function. **PancakeByalanLP.sol** ```solidity=1310 function harvest() external override whenNotPaused onlyEOA gasThrottle { IMasterChef(MASTERCHEF).deposit(pid, 0); chargeFees(); addLiquidity(); deposit(); emit Harvest(msg.sender); } ``` The `chargeFees()` function swaps the $CAKE reward to $BNB, split them to 3 portions, and send them to the respective fee collectors. **PancakeByalanLP.sol** ```solidity=1320 function chargeFees() internal nonReentrant { uint256 toBnb = (IERC20(CAKE).balanceOf(address(this)) * totalFee) / MAX_FEE; IUniswapV2Router02(unirouter).swapExactTokensForETH(toBnb, 0, cakeToWbnbRoute, address(this), block.timestamp); uint256 bnbBal = address(this).balance; uint256 callFeeAmount = (bnbBal * callFee) / MAX_FEE; payable(msg.sender).sendValue(callFeeAmount); uint256 treasuryFeeAmount = (bnbBal * treasuryFee) / MAX_FEE; payable(treasuryFeeRecipient).sendValue(treasuryFeeAmount); uint256 kswFeeAmount = (bnbBal * kswFee) / MAX_FEE; payable(kswFeeRecipient).sendValue(kswFeeAmount); } ``` The 3 portions of the fees can be set to be any value by the accounts allowed in the `onlyManager` modifier, including the contract owner. **PancakeByalanLP.sol** ```solidity=1163 function setCallFee(uint256 _fee) external onlyManager { callFee = _fee; } function setTreasuryFee(uint256 _fee) external onlyManager { treasuryFee = _fee; } function setKSWFee(uint256 _fee) external onlyManager { kswFee = _fee; } ``` This means that if the sum of the fees exceeds `MAX_FEE`, there will be insufficient $BNB to transfer, causing the transaction to be reverted. And on the other hand, if the sum of the fees is less than `MAX_FEE`, not all $BNB will be transferred, and some will be stuck in the contract. #### Remediation Inspex suggests adding a variable to store the sum of 3 fees, and use that sum as the denominator in the fee portion calculation, for example: - Add the sum storage variable: **PancakeByalanLP.sol** ```solidity=1148 abstract contract Sailor is ByalanIsland, ISailor { uint256 public constant override MAX_FEE = 10000; uint256 public override totalFee = 300; // 3% uint256 public constant MAX_TOTAL_FEE = 1000; // 10% uint256 public override callFee = 4000; // 40% of total fee uint256 public treasuryFee = 3000; // 30% of total fee uint256 public override kswFee = 3000; // 30% of total fee uint256 public feeSum = 10000; ``` - Update the sum on each modification of the fee: **PancakeByalanLP.sol** ```solidity=1163 function setCallFee(uint256 _fee) external onlyManager { callFee = _fee; feeSum = callFee + treasuryFee + kswFee; } function setTreasuryFee(uint256 _fee) external onlyManager { treasuryFee = _fee; feeSum = callFee + treasuryFee + kswFee; } function setKSWFee(uint256 _fee) external onlyManager { kswFee = _fee; feeSum = callFee + treasuryFee + kswFee; } } ``` - Use the sum in the calculation of the fee amount: **PancakeByalanLP.sol** ```solidity=1320 function chargeFees() internal nonReentrant { uint256 toBnb = (IERC20(CAKE).balanceOf(address(this)) * totalFee) / MAX_FEE; IUniswapV2Router02(unirouter).swapExactTokensForETH(toBnb, 0, cakeToWbnbRoute, address(this), block.timestamp); uint256 bnbBal = address(this).balance; uint256 callFeeAmount = (bnbBal * callFee) / feeSum; payable(msg.sender).sendValue(callFeeAmount); uint256 treasuryFeeAmount = (bnbBal * treasuryFee) / feeSum; payable(treasuryFeeRecipient).sendValue(treasuryFeeAmount); uint256 kswFeeAmount = (bnbBal * kswFee) / feeSum; payable(kswFeeRecipient).sendValue(kswFeeAmount); } ``` ### IDX-011 Improper Fee Setting (Deposit and Withdraw Fee) - **Target:** FeeKafra - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-755: Improper Handling of Exceptional Conditions - **Severity:** Low - **Impact:** Medium The `_deposit()` function of `contract` will be unusable if the fee is set incorrectly. - **Likelihood:** Low It is unlikely that the owner will set the fees incorrectly and leave them unfixed. - **Status:** Open #### Description In the `Izlude` contract, `_deposit()` function can be used to deposit `want` token to the contract and `_withdraw()` function can be used to withdraw `want` token from the contract. Some fees are deducted from deposit and withdrawal amount as deposit fee and withdraw fee with `feeKafra.distributeDepositFee()` and `feeKafra.distributeWithdrawFee()`. **Izlude.sol** ```solidity=810 function _deposit(uint256 _amount, address _user) private nonReentrant returns (uint256) { byalan.beforeDeposit(); uint256 _pool = balance(); want().safeTransferFrom(_msgSender(), address(this), _amount); uint256 _after = balance(); _amount = _after - _pool; uint256 fee = calculateDepositFee(_amount, _user); if (fee > 0) { want().safeTransfer(address(feeKafra), fee); feeKafra.distributeDepositFee(want(), _user); } earn(); _after = balance(); _amount = _after - _pool; // Additional check for deflationary tokens require( address(allocKafra) == address(0) || allocKafra.canAllocate(_amount, byalan.balanceOf(), byalan.balanceOfMasterChef(), _user), "capacity limit reached" ); uint256 jellopy = 0; if (totalSupply() == 0) { jellopy = _amount; } else { jellopy = (_amount * totalSupply()) / _pool; } _mint(_user, jellopy); return jellopy; } ``` **Izlude.sol** ```solidity=862 function _withdraw(uint256 _jellopy, address _user) private nonReentrant returns (uint256) { uint256 r = (balance() * _jellopy) / totalSupply(); _burn(_user, _jellopy); uint256 b = want().balanceOf(address(this)); if (b < r) { uint256 amount = r - b; byalan.withdraw(amount); uint256 _after = want().balanceOf(address(this)); uint256 diff = _after - b; if (diff < amount) { r = b + diff; } } uint256 fee = calculateWithdrawFee(r, _user); if (fee > 0) { r -= fee; want().safeTransfer(address(feeKafra), fee); feeKafra.distributeWithdrawFee(want(), _user); } want().safeTransfer(_msgSender(), r); return r; } ``` The fees in `feeKafra.calculateDepositFee()` and `feeKafra.calculateWithdrawFee()` are split into 2 portions; `treasuryFeeAmount` and `kswFeeAmount`. **FeeKafra.sol** ```solidity=605 function distributeDepositFee(IERC20 _token, address) external override { uint256 feeAmount = _token.balanceOf(address(this)); uint256 treasuryFeeAmount = (feeAmount * treasuryFeeDeposit) / MAX_FEE; if (treasuryFeeAmount > 0) { _token.safeTransfer(treasuryFeeRecipient, treasuryFeeAmount); } uint256 kswFeeAmount = (feeAmount * kswFeeDeposit) / MAX_FEE; if (kswFeeAmount > 0) { _token.safeTransfer(kswFeeRecipient, kswFeeAmount); } } function distributeWithdrawFee(IERC20 _token, address) external override { uint256 feeAmount = _token.balanceOf(address(this)); uint256 treasuryFeeAmount = (feeAmount * treasuryFeeWithdraw) / MAX_FEE; if (treasuryFeeAmount > 0) { _token.safeTransfer(treasuryFeeRecipient, treasuryFeeAmount); } uint256 kswFeeAmount = (feeAmount * kswFeeWithdraw) / MAX_FEE; if (kswFeeAmount > 0) { _token.safeTransfer(kswFeeRecipient, kswFeeAmount); } } ``` The 2 portions of the fees can be set to be any value by the contract owner. **FeeKafra.sol** ```solidity=581 function setTreasuryFeeDeposit(uint256 _fee) external onlyOwner { treasuryFeeDeposit = _fee; } function setKSWFeeDeposit(uint256 _fee) external onlyOwner { kswFeeDeposit = _fee; } function setTreasuryFeeWithdraw(uint256 _fee) external onlyOwner { treasuryFeeWithdraw = _fee; } function setKSWFeeWithdraw(uint256 _fee) external onlyOwner { kswFeeWithdraw = _fee; } ``` This means that if the sum of the fees exceeds `MAX_FEE`, there will be insufficient `want` token to transfer, causing the transaction to be reverted. And on the other hand, if the sum of the fees is less than `MAX_FEE`, not all `want` token will be transferred, and some will be stuck in the contract. #### Remediation Inspex suggests adding a variable to store the sum of 2 fees, and use that sum as the denominator in the fee portion calculation, for example: - Add the sum storage variable: **FeeKafra.sol** ```solidity=538 contract FeeKafra is Ownable, IFeeKafra { using SafeERC20 for IERC20; uint256 public constant override MAX_FEE = 10000; uint256 public constant MAX_DEPOSIT_FEE = 1000; // 10% uint256 public override depositFee = 0; // 2 % uint256 public override treasuryFeeDeposit = 5000; // 50% of total fee uint256 public override kswFeeDeposit = 5000; // 50% of total fee uint256 public depositFeeSum = 10000; uint256 public constant MAX_WITHDRAW_FEE = 1000; // 10% uint256 public override withdrawFee = 200; // 2 % uint256 public override treasuryFeeWithdraw = 5000; // 50% of total fee uint256 public override kswFeeWithdraw = 5000; // 50% of total fee uint256 public withdrawFeeSum = 10000; ``` - Update the sum on each modification of the fee: **FeeKafra.sol** ```solidity=581 function setTreasuryFeeDeposit(uint256 _fee) external onlyOwner { treasuryFeeDeposit = _fee; depositFeeSum = treasuryFeeDeposit + kswFeeDeposit; } function setKSWFeeDeposit(uint256 _fee) external onlyOwner { kswFeeDeposit = _fee; depositFeeSum = treasuryFeeDeposit + kswFeeDeposit; } function setTreasuryFeeWithdraw(uint256 _fee) external onlyOwner { treasuryFeeWithdraw = _fee; withdrawFeeSum = treasuryFeeWithdraw + kswFeeWithdraw; } function setKSWFeeWithdraw(uint256 _fee) external onlyOwner { kswFeeWithdraw = _fee; withdrawFeeSum = treasuryFeeWithdraw + kswFeeWithdraw; } ``` - Use the sum in the calculation of the fee amount: **FeeKafra.sol** ```solidity=605 function distributeDepositFee(IERC20 _token, address) external override { uint256 feeAmount = _token.balanceOf(address(this)); uint256 treasuryFeeAmount = (feeAmount * treasuryFeeDeposit) / depositFeeSum; if (treasuryFeeAmount > 0) { _token.safeTransfer(treasuryFeeRecipient, treasuryFeeAmount); } uint256 kswFeeAmount = (feeAmount * kswFeeDeposit) / depositFeeSum; if (kswFeeAmount > 0) { _token.safeTransfer(kswFeeRecipient, kswFeeAmount); } } function distributeWithdrawFee(IERC20 _token, address) external override { uint256 feeAmount = _token.balanceOf(address(this)); uint256 treasuryFeeAmount = (feeAmount * treasuryFeeWithdraw) / withdrawFeeSum; if (treasuryFeeAmount > 0) { _token.safeTransfer(treasuryFeeRecipient, treasuryFeeAmount); } uint256 kswFeeAmount = (feeAmount * kswFeeWithdraw) / withdrawFeeSum; if (kswFeeAmount > 0) { _token.safeTransfer(kswFeeRecipient, kswFeeAmount); } } ``` ### IDX-012 Design Flaw in massUpdatePools() Function - **Target:** Prontera - **Category:** General Smart Contract Vulnerability - **CWE:** CWE-400: Uncontrolled Resource Consumption - **Severity:** Low - **Impact:** Medium The `massUpdatePools()` function will eventually be unusable due to excessive gas usage. - **Likelihood:** Low It is very unlikely that the `poolInfo` size will be raised until the `massUpdatePools()` is eventually unusable. - **Status:** Open #### Description The `massUpdatePools()` function executes the `updatePool()` function, which is a state modifying function for all added pools as shown below: **Morroc.sol** ```solidity=1747 function massUpdatePools() public { uint256 length = poolInfo.length; for (uint256 pid = 0; pid < length; ++pid) { updatePool(pid); } } ``` With the current design, the added pools cannot be removed. They can only be disabled by setting the `pool.allocPoint` to 0. Even if a pool is disabled, the `updatePool()` function for this pool is still called. Therefore, if new pools continue to be added to this contract, the `poolInfo.length` will continue to grow and this function will eventually be unusable due to excessive gas usage. #### Remediation Inspex suggests making the contract capable of removing unnecessary or ended pools to reduce the loop round in the `massUpdatePools()` function. ### IDX-013 Dangerous Approval to External Contract - **Target:** - PancakeByalanLP - PancakeAlbertaBNBLP - PancakeAlbertaBUSDLP - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-284: Improper Access Control - **Severity:** Very Low - **Impact:** Low The external spender contracts can steal all approved tokens from the approver contract, but amount of token left in the contract is very low. - **Likelihood:** Low It is very unlikely that the external contracts specifically defined by the owner will steal tokens from the contract. - **Status:** Open #### Description Giving excessive token allowance to external contracts could cause serious damages since the external contracts can spend or take more tokens from the contract than intended. The `PancakeByalanLP` contract, the `PancakeAlbertaBNBLP` contract, and the `PancakeAlbertaBUSDLP` contract contain functions that approve excessive allowances to external contracts, which could be represented as followed: **Scenario 1: the `PancakeByalanLP` contract's allowance** The `_giveAllowances()` function is used for allowing `MASTERCHEF` and `unirouter` to spend arbitrary amount of LP tokens (`want`), `$CAKE`, and other tokens (`lpToken0` and `lpToken1`). **PancakeByalanBP.sol** ```solidity=1427 function _giveAllowances() internal { IERC20(want).safeApprove(MASTERCHEF, type(uint256).max); IERC20(CAKE).safeApprove(unirouter, type(uint256).max); // lp token 0 and 1 maybe cake so approve 0 is needed here IERC20(lpToken0).safeApprove(unirouter, 0); IERC20(lpToken0).safeApprove(unirouter, type(uint256).max); IERC20(lpToken1).safeApprove(unirouter, 0); IERC20(lpToken1).safeApprove(unirouter, type(uint256).max); } ``` This function is executed since it was initialized (from constructor) as in line 1277. **PancakeByalanBP.sol** ```solidity=1243 constructor( address _hydra, address _izlude, address _kswFeeRecipient, address _treasuryFeeRecipient, uint256 _pid ) ByalanIsland( _hydra, 0x10ED43C718714eb63d5aA57B78B54704E256024E, _izlude, _kswFeeRecipient, _treasuryFeeRecipient ) { pid = _pid; want = IMasterChef(MASTERCHEF).poolInfo(_pid).lpToken; lpToken0 = IUniswapV2Pair(want).token0(); lpToken1 = IUniswapV2Pair(want).token1(); cakeToWbnbRoute = [CAKE, WBNB]; if (lpToken0 == WBNB) { cakeToLp0Route = [CAKE, WBNB]; } else if (lpToken0 != CAKE) { cakeToLp0Route = [CAKE, WBNB, lpToken0]; } if (lpToken1 == WBNB) { cakeToLp1Route = [CAKE, WBNB]; } else if (lpToken1 != CAKE) { cakeToLp1Route = [CAKE, WBNB, lpToken1]; } _giveAllowances(); } ``` **Scenario 2: the `PancakeAlbertaBNBLP` contract's allowance** There are the `add()` function and the `remove()` function which give max allowance to the `ROUTER` address as in line 1101 and line 1109, for example: **PancakeAlbertaBNBLP.sol** ```solidity=1088 function remove( address _paidToken, address _wantToken, address _to, uint256 _amountIn, uint256 _maxPriceImpact ) external payable override nonReentrant { IERC20(_wantToken).safeTransferFrom(msg.sender, address(this), _amountIn); IUniswapV2Pair lpToken = IUniswapV2Pair(_wantToken); address otherToken = lpToken.token0() == WBNB ? lpToken.token1() : lpToken.token0(); // Remove all liquidity back to BNB and farming tokens. lpToken.approve(address(ROUTER), type(uint256).max); ROUTER.removeLiquidityETH(otherToken, lpToken.balanceOf(address(this)), 0, 0, address(this), block.timestamp); // Convert farming tokens to BNB. address[] memory path = new address[](2); (path[0], path[1]) = (otherToken, WBNB); IERC20(otherToken).safeApprove(address(ROUTER), 0); IERC20(otherToken).safeApprove(address(ROUTER), type(uint256).max); uint256 amountIn = IERC20(otherToken).balanceOf(address(this)); _safePriceImpactSwap(amountIn, path, _maxPriceImpact); ROUTER.swapExactTokensForETH(amountIn, 0, path, address(this), block.timestamp); // Return all back to the original caller. if (_paidToken != address(0)) { address[] memory pathToPaid = new address[](2); (pathToPaid[0], pathToPaid[1]) = (WBNB, _paidToken); _safePriceImpactSwap(address(this).balance, pathToPaid, _maxPriceImpact); ROUTER.swapExactETHForTokens{value: address(this).balance}(0, pathToPaid, _to, block.timestamp); } else { payable(_to).sendValue(address(this).balance); } } ``` **Scenario 3: the `PancakeAlbertaBUSDLP` contract's allowance** There are the `add()` function and the `remove()` function which give max allowance to the `ROUTER` address as in line 1107 and line 1123, for example: **PancakeAlbertaBUSDLP.sol** ```solidity=1094 function remove( address _paidToken, address _wantToken, address _to, uint256 _amountIn, uint256 _maxPriceImpact ) external payable override nonReentrant { IERC20(_wantToken).safeTransferFrom(msg.sender, address(this), _amountIn); IUniswapV2Pair lpToken = IUniswapV2Pair(_wantToken); address otherToken = lpToken.token0() == BUSD ? lpToken.token1() : lpToken.token0(); // Remove all liquidity back to BUSD and farming tokens. lpToken.approve(address(ROUTER), type(uint256).max); ROUTER.removeLiquidity( BUSD, otherToken, lpToken.balanceOf(address(this)), 0, 0, address(this), block.timestamp ); // Convert farming tokens to BUSD. address[] memory pathToBusd = new address[](2); (pathToBusd[0], pathToBusd[1]) = (otherToken, BUSD); IERC20(otherToken).safeApprove(address(ROUTER), 0); IERC20(otherToken).safeApprove(address(ROUTER), type(uint256).max); uint256 otherBal = IERC20(otherToken).balanceOf(address(this)); _safePriceImpactSwap(otherBal, pathToBusd, _maxPriceImpact); ROUTER.swapExactTokensForTokens(otherBal, 0, pathToBusd, address(this), block.timestamp); // Return all back to the original caller. if (_paidToken == BUSD) { IERC20(BUSD).safeTransfer(_to, IERC20(BUSD).balanceOf(address(this))); } else if (_paidToken == address(0)) { // paid == BNB address[] memory pathToBnb = new address[](2); pathToBnb[0] = BUSD; pathToBnb[1] = WBNB; uint256 busdBal = IERC20(BUSD).balanceOf(address(this)); _safePriceImpactSwap(busdBal, pathToBnb, _maxPriceImpact); ROUTER.swapExactTokensForETH(busdBal, 0, pathToBnb, _to, block.timestamp); } else { // busd to paid token address[] memory pathToPaid = new address[](3); (pathToPaid[0], pathToPaid[1], pathToPaid[2]) = (BUSD, WBNB, _paidToken); uint256 busdBal = IERC20(BUSD).balanceOf(address(this)); _safePriceImpactSwap(busdBal, pathToPaid, _maxPriceImpact); ROUTER.swapExactTokensForTokens(busdBal, 0, pathToPaid, _to, block.timestamp); } } ``` #### Remediation Inspex suggests that the contracts should approve only the required amount of allowances to the external contracts. This can be achieved by using `SafeERC20`'s `safeIncreaseAllowance()` function with the sufficient amount before executing the external contracts. The scenarios below are described to demonstrate how to approve the allowances: **Scenario 1: the `PancakeByalanLP` contract's allowance** The `PancakeByalanLP` contract spends tokens in the following functions: - `deposit()` - `withdraw()` - `chargesFee()` - `addLiquidity()` - `retireStrategy()` The `_giveAllowances()` function should be removed, and `safeIncreaseAllowance()` should be used before executing external contract calling instead. For example: **PancakeByalanLP.sol** ```solidity=1286 function deposit() public override whenNotPaused { uint256 wantBal = IERC20(want).balanceOf(address(this)); if (wantBal > 0) { uint256 curAllowance = IERC20(want).allowance(address(this), MASTERCHEF); // insufficient allowance, need more if (curAllowance < wantBal) { uint256 needAmount = wantBal - curAllowance; IERC20(want).safeIncreaseAllowance(MASTERCHEF, needAmount); } IMasterChef(MASTERCHEF).deposit(pid, wantBal); IERC20(want).safeApprove(MASTERCHEF, 0); } } ``` **Scenario 2: the `PancakeAlbertaBNBLP` contract's allowance** The `PancakeAlbertaBNBLP` contract spends tokens in the following functions: - `add()` function - `remove()` function Hence, the `safeIncreaseAllowance()` function should be used before before executing external contract calling instead. For example: **PancakeAlbertaBNBLP.sol** ```solidity=1088 function remove( address _paidToken, address _wantToken, address _to, uint256 _amountIn, uint256 _maxPriceImpact ) external payable override nonReentrant { IERC20(_wantToken).safeTransferFrom(msg.sender, address(this), _amountIn); IUniswapV2Pair lpToken = IUniswapV2Pair(_wantToken); address otherToken = lpToken.token0() == WBNB ? lpToken.token1() : lpToken.token0(); uint256 lpAllowance = IERC20(lpToken).allowance(address(this), ROUTER); // check current allowance if (lpAllowance < lpToken.balanceOf(address(this))) { uint256 needLpAmount = IERC20(lpToken).balanceOf(address(this)) - lpAllowance; IERC20(lpToken).safeIncreaseAllowance(ROUTER, needLpAmount); } // Remove all liquidity back to BNB and farming tokens. ROUTER.removeLiquidityETH(otherToken, lpToken.balanceOf(address(this)), 0, 0, address(this), block.timestamp); // Convert farming tokens to BNB. address[] memory path = new address[](2); (path[0], path[1]) = (otherToken, WBNB); uint256 amountIn = IERC20(otherToken).balanceOf(address(this)); uint256 otherAllowance = IERC20(otherToken).allowance(address(this), ROUTER); // check current allowance if (otherAllowance < amountIn) { uint256 needOtherAmount = amountIn - otherAllowance; IERC20(otherToken).safeIncreaseAllowance(address(ROUTER), needOtherAmount); } _safePriceImpactSwap(amountIn, path, _maxPriceImpact); ROUTER.swapExactTokensForETH(amountIn, 0, path, address(this), block.timestamp); // Return all back to the original caller. if (_paidToken != address(0)) { address[] memory pathToPaid = new address[](2); (pathToPaid[0], pathToPaid[1]) = (WBNB, _paidToken); _safePriceImpactSwap(address(this).balance, pathToPaid, _maxPriceImpact); ROUTER.swapExactETHForTokens{value: address(this).balance}(0, pathToPaid, _to, block.timestamp); } else { payable(_to).sendValue(address(this).balance); } IERC20(lpToken).safeApprove(address(ROUTER), 0); IERC20(otherToken).safeApprove(address(ROUTER), 0); } ``` Note: Other issues' remediations are not yet applied to the example above. **Scenario 3: the `PancakeAlbertaBUSDLP` contract's allowance** The `PancakeAlbertaBUSDLP` contract spends tokens in the following functions: - `add()` - `remove()` Hence, the `safeIncreaseAllowance()` function should be used before before executing external contract calling instead. For example: **PancakeAlbertaBUSDLP.sol** ```solidity=1094 function remove( address _paidToken, address _wantToken, address _to, uint256 _amountIn, uint256 _maxPriceImpact ) external payable override nonReentrant { IERC20(_wantToken).safeTransferFrom(msg.sender, address(this), _amountIn); IUniswapV2Pair lpToken = IUniswapV2Pair(_wantToken); address otherToken = lpToken.token0() == BUSD ? lpToken.token1() : lpToken.token0(); uint256 lpAllowance = IERC20(lpToken).allowance(address(this), ROUTER); // check current allowance if (lpAllowance < lpToken.balanceOf(address(this))) { uint256 needLpAmount = IERC20(lpToken).balanceOf(address(this)) - lpAllowance; IERC20(lpToken).safeIncreaseAllowance(ROUTER, needLpAmount); } // Remove all liquidity back to BUSD and farming tokens. ROUTER.removeLiquidity( BUSD, otherToken, lpToken.balanceOf(address(this)), 0, 0, address(this), block.timestamp ); // Convert farming tokens to BUSD. address[] memory pathToBusd = new address[](2); (pathToBusd[0], pathToBusd[1]) = (otherToken, BUSD); uint256 otherBal = IERC20(otherToken).balanceOf(address(this)); uint256 otherAllowance = IERC20(otherToken).allowance(address(this), ROUTER); // check current allowance if (otherAllowance < otherBal) { uint256 needOtherAmount = otherBal - otherAllowance; IERC20(otherToken).safeIncreaseAllowance(address(ROUTER), needOtherAmount); } _safePriceImpactSwap(otherBal, pathToBusd, _maxPriceImpact); ROUTER.swapExactTokensForTokens(otherBal, 0, pathToBusd, address(this), block.timestamp); // Return all back to the original caller. if (_paidToken == BUSD) { IERC20(BUSD).safeTransfer(_to, IERC20(BUSD).balanceOf(address(this))); } else if (_paidToken == address(0)) { // paid == BNB address[] memory pathToBnb = new address[](2); pathToBnb[0] = BUSD; pathToBnb[1] = WBNB; uint256 busdBal = IERC20(BUSD).balanceOf(address(this)); _safePriceImpactSwap(busdBal, pathToBnb, _maxPriceImpact); ROUTER.swapExactTokensForETH(busdBal, 0, pathToBnb, _to, block.timestamp); } else { // busd to paid token address[] memory pathToPaid = new address[](3); (pathToPaid[0], pathToPaid[1], pathToPaid[2]) = (BUSD, WBNB, _paidToken); uint256 busdBal = IERC20(BUSD).balanceOf(address(this)); _safePriceImpactSwap(busdBal, pathToPaid, _maxPriceImpact); ROUTER.swapExactTokensForTokens(busdBal, 0, pathToPaid, _to, block.timestamp); } IERC20(lpToken).safeApprove(address(ROUTER), 0); IERC20(otherToken).safeApprove(address(ROUTER), 0); } ``` Note: Other issues' remediations are not yet applied to the example above. ### IDX-014 Insufficient Logging for Privileged Functions - **Target:** - AllocKafra - FeeKafra - GasThrottler - Izlude - PronteraCastle - Prontera - PancakeAlbertaBNBLP - PancakeAlbertaBUSDLP - PancakeByalanLP - **Category:** Advanced Smart Contract Vulnerability - **CWE:** CWE-778: Insufficient Logging - **Severity:** Very Low - **Impact:** Low Privileged function executions cannot be monitored easily by the public. - **Likelihood:** Low It is not likely that most executions of the privileged functions are malicious actions. - **Status:** Open #### Description Privileged functions that are executable by the controlling parties are not logged properly by emitting events. Without events, it is not easy for the public to monitor the execution of those privileged functions, allowing the controlling parties to perform actions that cause big impacts to the platform. For example, the owner can increase the fee by executing the `setTotalFee()` function inherited from the `Sailor` contract in the `PancakeByalanLP` contract, and no event will be emitted. **PancakeByalanLP.sol** ```solidity=1157 function setTotalFee(uint256 _totalFee) external onlyManager { require(_totalFee <= MAX_TOTAL_FEE, "!cap"); totalFee = _totalFee; } ``` The privileged functions without sufficient logging are as follows: | File | Contract | Function | | --------------------------------- | -------------------- | ------------------------- | | AllocKafra.sol (L:148) | AllocKafra | setLimitAllocation() | | FeeKafra.sol (L:569) | FeeKafra | setDepositFee() | | FeeKafra.sol (L:575) | FeeKafra | setWithdrawFee() | | FeeKafra.sol (L:581) | FeeKafra | setTreasuryFeeDeposit() | | FeeKafra.sol (L:585) | FeeKafra | setKSWFeeDeposit() | | FeeKafra.sol (L:589) | FeeKafra | setTreasuryFeeWithdraw() | | FeeKafra.sol (L:593) | FeeKafra | setKSWFeeWithdraw() | | FeeKafra.sol (L:597) | FeeKafra | setKSWFeeRecipient() | | FeeKafra.sol (L:601) | FeeKafra | setTreasuryFeeRecipient() | | GasThrottler.sol (L:118) | GasThrottler | setGasPrice() | | Izlude.sol (L:728) | Izlude | setMorroc() | | Izlude.sol (L:732) | Izlude | setFeeKafra() | | Izlude.sol (L:736) | Izlude | setAllocKafra() | | Izlude.sol (L:909) | Izlude | upgradeStrategy() | | Izlude.sol (L:928) | Izlude | inCaseTokensGetStuck() | | Morroc.sol (L:1455) | PronteraCastle | setProntera() | | Morroc.sol (L:1675) | Prontera | add() | | Morroc.sol (L:1703) | Prontera | set() | | PancakeAlbertaBNBLP.sol (L:1148) | PancakeAlbertaBNBLP | recover() | | PancakeAlbertaBNBLP.sol (L:1152) | PancakeAlbertaBNBLP | recoverToken() | | PancakeAlbertaBUSDLP.sol (L:1173) | PancakeAlbertaBUSDLP | recover() | | PancakeAlbertaBUSDLP.sol (L:1177) | PancakeAlbertaBUSDLP | recoverToken() | | PancakeByalanLP.sol (L:1120) | ByalanIsland | setHydra() | | PancakeByalanLP.sol (L:1124) | ByalanIsland | setUnirouter() | | PancakeByalanLP.sol (L:1128) | ByalanIsland | setIzlude() | | PancakeByalanLP.sol (L:1132) | ByalanIsland | setTreasuryFeeRecipient() | | PancakeByalanLP.sol (L:1136) | ByalanIsland | setKswFeeRecipient() | | PancakeByalanLP.sol (L:1396) | PancakeByalanLP | retireStrategy() | | PancakeByalanLP.sol (L:1404) | PancakeByalanLP | panic() | #### Remediation Inspex suggests emitting events for the execution of privileged functions, for example: **PancakeByalanLP.sol** ```solidity=1157 event SetTotalFee(uint256 newTotalFee); function setTotalFee(uint256 _totalFee) external onlyManager { require(_totalFee <= MAX_TOTAL_FEE, "!cap"); totalFee = _totalFee; emit SetTotalFee(totalFee); } ``` ### IDX-015 Improper Reentrancy Attack Protection Mechanism - **Target:** Morroc - **Category:** General Smart Contract Vulnerability - **CWE:** CWE-841: Improper Enforcement of Behavioral Workflow - **Severity:** Info - **Impact:** None - **Likelihood:** None - **Status:** Open #### Description The `nonReentrant` modifier which can be applied to functions to make sure there are no nested calls to them. However, some functions which can be called from the hijacked control flow do not have the `nonReentrant` modifier applied, allowing reentrancy from another function. For example, the `withdrawToken()` function takes `_token` parameter from the user who calls the function. Users can set any contract address into the `withdrawToken()` function and the `balanceOf()` function of the malicious contract contract can be executed, allowing that contract to control the execution flow and call other functions. **Morroc.sol** ```solidity=1871 function withdrawToken( address _token, uint256 _jellopy, uint256 _maxPriceImpact ) public { uint256 beforeWantBal = izlude.want().balanceOf(address(this)); _withdraw(_jellopy); uint256 afterWantBal = izlude.want().balanceOf(address(this)); uint256 beforeBal = IERC20(_token).balanceOf(address(this)); alberta.remove(_token, address(izlude.want()), address(this), afterWantBal - beforeWantBal, _maxPriceImpact); uint256 afterBal = IERC20(_token).balanceOf(address(this)); IERC20(_token).safeTransfer(_msgSender(), afterBal - beforeBal); } ``` In this case, there is no impact from the external call of `balanceOf()` function because no token is stored in the `Morroc` contract and there is no function that can be called that allows the caller to benefit. The functions that the execution flow can be hijacked are show in the following table: | File | Contract | Function | | ------------------------------- | -------- | ------------------ | | Morroc.sol (L:1861) | Morroc | depositEther() | | Morroc.sol (L:1871) | Morroc | depositToken() | | Morroc.sol (L:1912) | Morroc | withdrawEther() | | Morroc.sol (L:1934) | Morroc | withdrawToken() | #### Remediation Inspex suggests applying `nonReentrant` modifier to all `public` and `external` visibility functions that can modify state variable. Please noted that the `nonReentrant` modifier can only be called once in a single transaction. Functions that are applied `nonReentrant` modifier will not be able to call the other functions that are also applied `nonReentrant` modifier. As can be seen in the following example, if `nonReentrant` modifier is applied to both `withdrawAll()` and `withdraw()` functions, the `withdrawAll()` function will be unusable. **Morroc.sol** ```solidity=1907 function withdrawAll() nonReentrant external { withdraw(izlude.balanceOf(_msgSender())); } ``` **Morroc.sol** ```solidity=1902 function withdraw(uint256 _jellopy) nonReentrant public { uint256 amount = _withdraw(_jellopy); izlude.want().transfer(_msgSender(), amount); } ``` Therefore, for this case, Inspex suggests applying `nonReentrant` only to `withdraw()` function. ### IDX-016 Improper Token Transfer - **Target:** - Morroc - PancakeByalanLP - **Category:** General Smart Contract Vulnerability - **CWE:** CWE-710: Improper Adherence to Coding Standard - **Severity:** Info - **Impact:** None - **Likelihood:** None - **Status:** Open #### Description In `Morroc` contract, the `withdraw()` function is used for transferring LP tokens (external contract) from the `izlude` contract to `_msgSender()`. **Morroc.sol** ```solidity=1902 function withdraw(uint256 _jellopy) public { uint256 amount = _withdraw(_jellopy); izlude.want().transfer(_msgSender(), amount); } ``` The table below represents a list of contracts and functions that uses the `transfer()` function. | Contract | Function | | --------------- | ---------------- | | Morroc | withdraw() | | PancakeByalanLP | retireStrategy() | #### Remediation Inspex suggests that the contracts should use `safeTransfer()` function from `SafeERC20` to ensure the token is transferred successfully, for example: **Morroc.sol** ```solidity=1902 using SafeERC20 for IERC20; function withdraw(uint256 _jellopy) public { uint256 amount = _withdraw(_jellopy); IERC20(izlude.want()).safeTransfer(_msgSender(), amount); } ``` ### IDX-017 Improper Function Visibility - **Target:** - FeeKafra - Izlude - PancakeByalanLP - **Category:** Smart Contract Best Practice - **CWE:** CWE-710: Improper Adherence to Coding Standards - **Severity:** Info - **Impact:** None - **Likelihood:** None - **Status:** Open #### Description Functions with public visibility copy calldata to memory when being executed, while external functions can read directly from calldata. Memory allocation uses more resources (gas) than reading directly from calldata. For example, the following source code shows that the `calculateDepositFee()` function of the `FeeKafra` contract is set to public and it is never called from any internal function. **FeeKafra.sol** ```solidity=561 function calculateDepositFee(uint256 _wantAmount, address) public view override returns (uint256) { return (_wantAmount * depositFee) / MAX_FEE; } ``` The following table contains all functions that have `public` visibility and are never called from any internal function. | File | Contract | Function | | ----------------------------- | --------------- | ---------------------- | | FeeKafra.sol (L: 561) | FeeKafra | calculateDepositFee() | | FeeKafra.sol (L: 565) | FeeKafra | calculateWithdrawFee() | | Izlude.sol (L: 744) | Izlude | balanceOf() | | PancakeByalanLP.sol (L: 1373) | PancakeByalanLP | balanceOf() | | PancakeByalanLP.sol (L: 1387) | PancakeByalanLP | balanceOfMasterChef() | | PancakeByalanLP.sol (L: 1404) | PancakeByalanLP | panic() | #### Remediation Inspex suggests changing all functions' visibility to `external` if they are not called from any `internal` function as shown in the following example: **FeeKafra.sol** ```solidity=561 function calculateDepositFee(uint256 _wantAmount, address) external view override returns (uint256) { return (_wantAmount * depositFee) / MAX_FEE; } ``` ### IDX-018 Inexplicit Solidity Compiler Version - **Target:** - AllocKafra - FeeKafra - GasPrice - Izlude - PronteraCastle - PronteraGuard - Prontera - Morroc - GasThrottler - ByalanIsland - Sailor - PancakeAlbertaBNBLP - PancakeAlbertaBUSDLP - PancakeByalanLP - **Category:** Smart Contract Best Practice - **CWE:** CWE-1104: Use of Unmaintained Third Party Components - **Severity:** Info - **Impact:** None - **Likelihood:** None - **Status:** Open #### Description The Solidity compiler versions declared in the smart contracts were not explicit. Each compilation may be done using different compiler versions, which may potentially result in compatibility issues, for example: **AllocKafra.sol** ```solidity=125 pragma solidity ^0.8.0; contract AllocKafra is Ownable, IAllocKafra { ``` The following table contains all targets which the inexplicit compiler version is declared. | Contract | Version | | -------------------- | ------- | | AllocKafra | ^0.8.0 | | FeeKafra | ^0.8.0 | | GasPrice | ^0.8.0 | | Izlude | ^0.8.0 | | PronteraCastle | ^0.8.0 | | PronteraGuard | ^0.8.0 | | Prontera | ^0.8.0 | | Morroc | ^0.8.0 | | GasThrottler | ^0.8.0 | | ByalanIsland | ^0.8.0 | | Sailor | ^0.8.0 | | PancakeAlbertaBNBLP | ^0.8.0 | | PancakeAlbertaBUSDLP | ^0.8.0 | | PancakeByalanLP | ^0.8.0 | #### Remediation Inspex suggests fixing the solidity compiler to the latest stable version. At the time of audit, the latest stable versions of Solidity compiler in major 0.8 is v0.8.6.