Blockrover - Audit

In the contract `Ownable` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and change the ownership of current contract.  In the contract `BlockRover` is inherited from `Ownable` beyond functions above, the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and change the configuration of current contract.  ---- The following content is based on the version [0x0d5d5a69bc33326ca03667954fafccd25247de62 (07/18)](https://goerli.etherscan.io/address/0x0d5d5a69bc33326ca03667954fafccd25247de62#code) In the contract `BlockRover`, the privileged functions of the role `_owner` is changed as below: - Removed the `updateRouterAndPair()` function. - Added the `withdrawStuckTokens()` function to withdraw the stuck ERC20 tokens in the contract. - Added the `clearStuckEthers()` function to withdraw the stuck native tokens in the contract.

All of the `ROVER` tokens are sent to the contract deployer or one or several externally-owned account (EOA) addresses. This is a centralization risk because the deployer or the owner(s) of the EOAs can distribute tokens without obtaining the consensus of the community. Any compromise to these addresses may allow a hacker to steal and sell tokens on the market, resulting in severe damage to the project.

In `swapForFees` function, the contract will distribute the fees based on the sell tax distribution(`sellTaxes`), however, `buyTaxes` is never being used. We would like to check with the team if any logic missing and if this is intended design.

In the `_transfer()` function, the contract will trigger `swapForFees()` if the current fee is more than 0. However, the current fees haven't been transferred into the contract yet. This situation results in some fees remaining in the contract, even after the swap is performed. Therefore, the `swapForFees` function is intrinsically tied to the total tokens accumulated in the contract rather than the per-transaction fee. Furthermore, the condition `fee > 0` may not be an ideal check, as the contract might already have accumulated enough tokens for a swap. In such cases, even if the next transaction does not involve a fee, the swap could be executed. However, due to the current condition, it has to wait for the next transaction that involves a fee, which could introduce unnecessary latency.

A sandwich attack may happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (executing before the target) a transaction to purchase one of the assets and make profits by backrunning (executing after the target) a transaction to sell the asset. The following functions are called without setting restrictions on slippage or minimum output amount, so transactions triggering these functions are vulnerable to sandwich attacks, especially when the input amount is large: - `router.swapExactTokensForETHSupportingFeeOnTransferTokens` - `router.addLiquidityETH`

The smart contract does not check or store the return value of an external call in a local or state variable, which may introduce vulnerabilities due to the unhandled outcome. ```solidity=355 router.addLiquidityETH{value: bnbAmount}( address(this), tokenAmount, 0, // slippage is unavoidable 0, // slippage is unavoidable address(0xdead), block.timestamp ); ```