STE security record

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 transfer or renounce the ownership.  ----- In the contract `NftMarketplace` 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 withdraw native tokens and ERC20 tokens from the contract and set `whitelists` role, cancel listings, and set `buyTimeStart`, `buyTimeEnd`, `awardAddress`, and `marketAddress`.  In the contract `NftMarketplace` the role `whitelists` has authority over the function shown in the diagram below. Any compromise to the `whitelists` account may allow the hacker to take advantage of this authority and set `startTime`, `endTime`, `totalShare`, and `star`.  ----- In the contract `Ste` 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 set whitelisted addresses that can transfer tokens without some restrictions and set `limited`, `uniswapV2Pair`, and `nodeFeeAddress`.  ------ In the contract `MasterChef` 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 add pools and set pool infos, `rewardToken`, `rewardPerSecond`. 

All of the STE tokens are sent to the contract deployer. This is a centralization risk because the deployer can distribute tokens without obtaining the consensus of the community. Any compromise to the address may allow a hacker to steal and sell tokens on the market, resulting in severe damage to the project.

The project design may not be compatible with non-standard ERC20 tokens, such as deflationary tokens or rebase tokens. The functions use `transferFrom()`/`transfer()` to move funds from the sender to the recipient but fail to verify if the received token amount matches the transferred amount. This could pose an issue with fee-on-transfer tokens, where the post-transfer balance might be less than anticipated, leading to balance inconsistencies. There might be subsequent checks for a second transfer, but an attacker might exploit leftover funds (such as those accidentally sent by another user) to gain unjustified credit.

The `_isDelLiquidityV1()` and `_isAddLiquidityV1()` functions are flawed. They fail to accurately determine whether a token transfer involves adding or removing liquidity, and their return values can be manipulated by an attacker to bypass fees. 1. The result may differ depending on whether the token is `token0` or `token1` in the pair. 2. The outcome may also vary if the caller is using a router or directly interacting with the pair’s `swap()`, `mint()`, or `burn()` functions. 3. An attacker can easily avoid fees by directly transferring more than 1000 of `token0` to the pair if `address(this)` is `token1` (and vice versa), which would cause `_isAddLiquidityV1()` to incorrectly return true. Due to these inaccuracies, callers might be able to circumvent paying the intended fee.

In the `add()` and `set()` functions, the flag `_withUpdate` determines if all the pools will be updated. This reliance might lead to significant loss of the reward. For illustration, assume we have only one pool with `pool.allocPoint == 50` and `totalAllocPoint == 50` at the beginning. Now we want to add another pool with `pool.allocPoint == 50`. There will be two scenarios on calculating the pool reward, Case 1: `_withUpdate` is set to `true`. - distribute the reward and update the pool. - add the given pool information Case 2: `_withUpdate` is set to `false`. - add the given pool information (Note: While we focused on the `add()` function, both the `add()` and `set()` functions update `totalAllocPoint`, which is used in calculation of pool rewards in the function `updatePool()` * In Case 1, reward for the first pool is updated in the call to `updatePool()` (`_currentRewardPerShare()`) where `rewards = (block.timestamp - pool.lastRewardTime) * rewardPerSecond * pool.allocPoint / totalAllocPoint / 20`. * In Case 2, an update `totalAllocPoint = totalAllocPoint.add(_allocPoint)` is done first. Then `updatePool()` calculates the reward for the first pool: `rewards = (block.timestamp - pool.lastRewardTime) * rewardPerSecond * pool.allocPoint / totalAllocPoint / 20`. Because the second pool is sharing rewards with the first one, the amount of reward for the first pool becomes half as much as that in the first case.

The `getNextPriceAndFee()` function in the `NftMarketplace` contract calculates transaction fees using the `getAstrFeeByU()` function, which directly references token reserves from a Uniswap pair (`_reserve0` and `_reserve1`). This method exposes the fee calculation to potential manipulation. Attackers can engage in large trades that shift these reserves just before a fee is calculated, enabling them to decrease or increase the fees artificially. This could result in significantly underpaid fees for services like token splitting or listing, or inflated fees through front-running. ```solidity=194 function getAstrFeeByU(uint256 u) internal view returns (uint256) { uint112 _reserve0; uint112 _reserve1; (_reserve0, _reserve1, ) = pair.getReserves(); uint256 fee = (_reserve1 * u) / _reserve0; return fee; } ```

The `getUFeeByAstr()` and `getAstrFeeByU()` functions in the `NftMarketplace` contract are implemented with identical logic, which suggests a potential oversight in the implementation. ```solidity function getUFeeByAstr(uint256 fee) internal view returns (uint256) { uint112 _reserve0; uint112 _reserve1; (_reserve0, _reserve1, ) = pair.getReserves(); uint256 feeU = (_reserve1 * fee) / _reserve0; return feeU; } function getAstrFeeByU(uint256 u) internal view returns (uint256) { uint112 _reserve0; uint112 _reserve1; (_reserve0, _reserve1, ) = pair.getReserves(); uint256 fee = (_reserve1 * u) / _reserve0; return fee; } ```

The `getAstrFeeByU()` function in the `NftMarketplace` contract determines transaction fees based on token reserves obtained from a Pancake pair, but it fails to verify the association of these reserves with specific tokens. This oversight can result in incorrect fee calculations if the reserves (`_reserve0` and `_reserve1`) do not correspond to the expected tokens in a consistent manner. Consequently, this can lead to incompatibility with certain token pairs. ```solidity function getAstrFeeByU(uint256 u) internal view returns (uint256) { uint112 _reserve0; uint112 _reserve1; (_reserve0, _reserve1, ) = pair.getReserves(); uint256 fee = (_reserve1 * u) / _reserve0; return fee; } ```

The contract is serving as the underlying entity to interact with one or more out-of-scope protocols. The scope of the audit treats third party entities as black boxes and assumes their functional correctness. However, in the real world, third parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of third parties can possibly create severe impacts, such as increasing fees of third parties, migrating to new LP pools, etc. In `NftMarketplace` contract: ```solidity import "./library/DateTime.sol"; IERC20 public astrErc20; IERC20 public lsErc20; IERC20 public usdtAddress; address public nftAddress; IPancakePair public pair; ``` In `MasterChef` contract: ```solidity import {SafeTransferLib,ERC20} from "./library/SafeTransferLib.sol"; address lpToken; // Address of LP token contract. IStake2 stake2; // Stake2 contract address, will be called when deposit/withdraw. ERC20 public rewardToken; ``` In `Ste` contract: ```solidity address public uniswapV2Pair; ```

```solidity=236 unchecked { uint256 rewards = (block.timestamp - pool.lastRewardTime) * rewardPerSecond * pool.allocPoint / totalAllocPoint / 20; index = rewards * _PRECISION / lpSupply; } ``` The expression `(block.timestamp - pool.lastRewardTime) * rewardPerSecond * pool.allocPoint / totalAllocPoint` may divide by zero. This may lead to reversion.