Optimism PEPE security record

In the `_transfer()` function the following logic was found for dealing with treasury fees and reward fees. ```solidity=1097 uint256 newBalance = address(this).balance - initialBalance; if((treasuryFeeOnBuy + treasuryFeeOnSell) > 0) { uint256 allocatedToWallets = address(this).balance * 70 / 100; bool success; uint256 mark = marketingAllocation * allocatedToWallets / allocation_denominator; uint256 opsA = opsAllocation * allocatedToWallets / allocation_denominator; if(mark > 0) {(success,) = marketingWallet.call{value: mark, gas: 35000}("");} if(opsA > 0) {(success,) = opWallet.call{value: opsA, gas: 35000}("");} } if((rewardsFeeOnBuy + rewardsFeeOnSell) > 0) { uint256 rewardBNB = newBalance * (rewardsFeeOnBuy + rewardsFeeOnSell) / bnbShare; swapAndSendDividends(rewardBNB); } ``` It was noted that the amount `allocatedToWallets` is always set to 70% of the BNB in the contract to allocate towards the marketing and operation wallet addresses. This percentage used to calculate the `allocatedToWallets` seems to have some relation to `treasuryFeeOnBuy` and `treasuryFeeOnSell`, however neither variable is used in the calculation of the `allocatedToWallets` amount. In relation to the amount calculated for rewards, the amount of `rewardBNB` is calculated using the `newBalance`, representing the newly swapped native tokens, as the base (unlike the value of `allocatedToWallets` which uses the total BNB contract balance) and multiplies this by the ratio of the reward fees to the `bnbShare` (or `(treasuryFeeOnBuy + treasuryFeeOnSell) + (rewardsFeeOnBuy + rewardsFeeOnSell)`). There seems to be a discrepancy between the bases in the calculations of the fees and rewards which could lead to the fewer rewards being distributed.

The following code will run only if `liquidityFeeOnBuy` or `liquidityFeeOnSell` are greater than 0. ```solidity=1074 if(liquidityFeeOnBuy + liquidityFeeOnSell > 0) { liquidityTokens = contractTokenBalance * (liquidityFeeOnBuy + liquidityFeeOnSell) / 100; swapAndLiquify(liquidityTokens); } ``` However, the default values `liquidityFeeOnBuy` or `liquidityFeeOnSell` are set to 0 and there is no way to update these values, therefore the above code block will never execute. This also means that function `swapAndLiquify()` will never execute as well. ```solidity constructor() payable ERC20("Optimism PEPE", "OPEPE") { rewardToken = 0xbA2aE424d960c26247Dd6c32edC70B295c744C43; // Doge liquidityFeeOnBuy = 0; // ... Omitted code liquidityFeeOnSell = 0; ``` It is also important to note that none of the fees are ever able to be updated after being set in the constructor.

The `swapAndLiquify()` function converts half of the `contractTokenBalance` tokens to BNB. The other half of the tokens and part of the converted BNB are deposited into the pool as liquidity. For every `swapAndLiquify()` function call, a small amount of BNB remains in the contract. This is because the price of the token drops after swapping the first half of the tokens into BNBs, and the other half of the tokens require less than the converted BNB to be paired with it when adding liquidity. Since the contract doesn't appear to provide a way to withdraw those BNBs, and they will be locked in the contract forever. ```solidity=1169 function swapAndLiquify(uint256 tokens) private { uint256 half = tokens / 2; uint256 otherHalf = tokens - half; uint256 initialBalance = address(this).balance; address[] memory path = new address[](2); path[0] = address(this); path[1] = uniswapV2Router.WETH(); uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens( half, 0, // accept any amount of ETH path, address(this), block.timestamp); uint256 newBalance = address(this).balance - initialBalance; uniswapV2Router.addLiquidityETH{value: newBalance}( address(this), otherHalf, 0, // slippage is unavoidable 0, // slippage is unavoidable DEAD, block.timestamp ); emit SwapAndLiquify(half, newBalance, otherHalf); } ``` *Note: Since the `liquidityFeeOnBuy` and `liquidityFeeOnSell` are set as 0, the function `swapAndLiquify` will not be used.*

When the `owner()` of the contract changes, the attributes that are assigned to the owner, more importantly the exclusion from fees is left unchanged. The new owner is not added to the `_isExcludedFromFees` mapping and the old owner still maintains its fee exclusion status.

It appears that the transfer function triggers `dividendTracker.process(gas)` function to distribute dividends to the receivers. However, it has been observed that in the case of a large number of token holders, the calls may be reverted due to high gas consumption. Considering the existing logic that allows individuals to receive dividends back using `withdrawDividend()`, it is possible that these code blocks may be unnecessary.

The transfers to the `marketingWallet` and `opWallet` return a `success` flag but this flag is never used or recorded to take any action when the transfers fail. ```solidity=1106 if(mark > 0) {(success,) = marketingWallet.call{value: mark, gas: 35000}("");} if(opsA > 0) {(success,) = opWallet.call{value: opsA, gas: 35000}("");} ``` Based on the current design, the transfers can potentially continue to fail without the reason being known and no funds would ever reach the `marketingWallet` and `opWallet`.

The variable `minimumTokenBalanceForDividends` is used as a minimum token threshold for the eligibility to receive dividends. This variable can be updated via the `updateMinimumTokenBalanceForDividends()` function. ```solidity=696 function updateMinimumTokenBalanceForDividends(uint256 _newMinimumBalance) external onlyOwner { require(_newMinimumBalance != minimumTokenBalanceForDividends, "New mimimum balance for dividend cannot be same as current minimum balance"); minimumTokenBalanceForDividends = _newMinimumBalance; } ``` However, when `minimumTokenBalanceForDividends` is changed, the users that can receive dividends is unchanged. If `minimumTokenBalanceForDividends` is increased, old users with a balance below the new `minimumTokenBalanceForDividends` but above the old `minimumTokenBalanceForDividends` can still receive dividends. ```solidity=802 function setBalance(address payable account, uint256 newBalance) external onlyOwner { if(excludedFromDividends[account]) { return; } if(newBalance >= minimumTokenBalanceForDividends) { _setBalance(account, newBalance); tokenHoldersMap.set(account, newBalance); } else { _setBalance(account, 0); tokenHoldersMap.remove(account); } ``` The audit team would like to confirm whether this is expected when changing the `minimumTokenBalanceForDividends` variable. If not, it would be advised to consider updating the list of dividend recipients when changing the `minimumTokenBalanceForDividends` variable.

In the highlighted try/catch statements there is no handling for the catch condition to handle error conditions. ```solidity=1147 try dividendTracker.setBalance(payable(from), balanceOf(from)) {} catch {} try dividendTracker.setBalance(payable(to), balanceOf(to)) {} catch {} ``` ```solidity=1153 try dividendTracker.process(gas) returns (uint256 iterations, uint256 claims, uint256 lastProcessedIndex) { emit ProcessedDividendTracker(iterations, claims, lastProcessedIndex, true, gas, tx.origin); } catch { } ``` If any of the transactions fail, there is no way to trace what failed and the reason for the failure since tokens are still being transferred between `from` and `to`. This would lead to discrepancies between the amounts transferred, the balance records on those transfers, and dividend records as well.

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: - `_transfer()` ```solidity=1090 uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens( contractTokenBalance, 0, path, address(this), block.timestamp); ``` - `swapAndLiquify()` ```solidity=1179 uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens( half, 0, // accept any amount of ETH path, address(this), block.timestamp); ``` - `swapAndSendDividends()` ```solidity=1179 uniswapV2Router.swapExactETHForTokensSupportingFeeOnTransferTokens{value: amount}( 0, path, address(this), block.timestamp ); ``` Using the deadline block.timestamp also does not help with preventing attacks since a malicious validator can potentially hold the transaction until the max slippage has occurred and the time that the transaction is included in a block will be the eventual `block.timestamp`. It might be preferable to include an actual inputted timestamp as a deadline.

### SafeMathUint ```solidity=142 int256 b = int256(a); ``` - The type conversion `int256(a)` from type uint256 to type int256 may flip the value's sign. ### IterableMapping ```solidity=164 return int(map.indexOf[key]); ``` - The type conversion `int256(map.indexOf[key])` from type uint256 to type int256 may flip the value's sign. ### DividendTracker ```solidity=748 iterationsUntilProcessed = index.sub(int256(lastProcessedIndex)); ``` - The type conversion `int256(lastProcessedIndex)` from type uint256 to type int256 may flip the value's sign. --- ```solidity=756 iterationsUntilProcessed = index.add(int256(processesUntilEndOfArray)); ``` - The type conversion `int256(processesUntilEndOfArray)` from type uint256 to type int256 may flip the value's sign.