Pepe Community security record

In the `ERC20` contract, function `transferFrom()` violates the "Check-Effects-Interactions" pattern, which is a best practice for writing secure smart contracts. The pattern suggests that all state changes should be performed before making any external function calls to prevent reentrancy attacks and ensure the predictable behavior of the contract. ```solidity=298 function transferFrom( address sender, address recipient, uint256 amount ) public virtual override returns (bool) { _transfer(sender, recipient, amount); _approve(sender, _msgSender(), _allowances[sender][_msgSender()].sub(amount, "ERC20: transfer amount exceeds allowance")); return true; } ``` In the `transferFrom()` function, the `_transfer()` function is called before the `_approve()` function. The `_transfer()` function is responsible for transferring tokens from the sender to the recipient, and it may involve external interactions such as swapping and adding liquidity. By calling `_transfer()` before `_approve()`, the function allows external calls to be made before completing the necessary state changes. This order of function calls can lead to a reentrancy vulnerability. An attacker could potentially exploit this vulnerability by creating a malicious contract that calls back into the `transferFrom()` function during the execution of the `_transfer()` function. This can result in unexpected behavior and potentially allow the attacker to manipulate the contract's state or steal tokens. It is also a better practice to validate the allowance first to reduce gas costs in case it reverts.

Performing integer division before multiplication truncates the low bits, losing the precision of calculation. ```solidity=1212 fees = amount.mul(sellTotalFees).div(100); ``` ```solidity=1213 tokensForLiquidity += fees * sellLiquidityFee / sellTotalFees; ```

If a reentrancy attack happened in the function `_transfer()` while the swap is occurring, the variable `swapping` could be `true` in Line#1200, then the `takeFee` would always be `false`. ```solidity=1200 bool takeFee = !swapping; ```

The following is the logic contained in the `swapBack()` function. ```solidity function swapBack() private { uint256 contractBalance = balanceOf(address(this)); uint256 totalTokensToSwap = tokensForLiquidity + tokensForMarketing + tokensForDev; bool success; if(contractBalance == 0 || totalTokensToSwap == 0) {return;} if(contractBalance > swapTokensAtAmount * 20){ contractBalance = swapTokensAtAmount * 20; } uint256 liquidityTokens = contractBalance * tokensForLiquidity / totalTokensToSwap / 2; uint256 amountToSwapForETH = contractBalance.sub(liquidityTokens); uint256 initialETHBalance = address(this).balance; swapTokensForEth(amountToSwapForETH); uint256 ethBalance = address(this).balance.sub(initialETHBalance); uint256 ethForMarketing = ethBalance.mul(tokensForMarketing).div(totalTokensToSwap); uint256 ethForDev = ethBalance.mul(tokensForDev).div(totalTokensToSwap); uint256 ethForLiquidity = ethBalance - ethForMarketing - ethForDev; tokensForLiquidity = 0; tokensForMarketing = 0; tokensForDev = 0; ``` If the condition `if(contractBalance > swapTokensAtAmount * 20)` is met, not all the tokens in the contract are transferred in this case, however, all the tokens for liquidity, marketing and dev are set to zero at the end of the function, which is incorrect. As a result, if fees are updated immediately, the tokens for liquidity, marketing and dev would be miscalculated.

The transaction may be charged fees, so the max holding of the receiver should be the sum of the held tokens and the amount received from the transfer. The fees should not be included in the max holding. ```solidity=1164 require(amount + balanceOf(to) <= maxWallet, "Max wallet exceeded"); ``` - Max holding is checked. ```solidity=1229 amount -= fees; ``` - Receiving amount is adjusted. ```solidity=369 _balances[recipient] = _balances[recipient].add(amount); ``` - Balance is increased by the adjusted amount.

Performing division by zero would raise an error and cause the transaction to revert. ```solidity=1213 tokensForLiquidity += fees * sellLiquidityFee / sellTotalFees; ``` The expression `fees * sellLiquidityFee / sellTotalFees` may divide by zero. Its divisor has has estimated interval [0, 99]. --- ```solidity=1214 tokensForDev += fees * sellDevFee / sellTotalFees; ``` The expression `fees * sellDevFee / sellTotalFees` may divide by zero. Its divisor has has estimated interval [0, 99]. --- ```solidity=1215 tokensForMarketing += fees * sellMarketingFee / sellTotalFees; ``` The expression `fees * sellMarketingFee / sellTotalFees` may divide by zero. Its divisor has has estimated interval [0, 99]. --- ```solidity=1220 tokensForLiquidity += fees * buyLiquidityFee / buyTotalFees; ``` The expression `fees * buyLiquidityFee / buyTotalFees` may divide by zero. Its divisor has has estimated interval [0, 50]. --- ```solidity=1221 tokensForDev += fees * buyDevFee / buyTotalFees; ``` The expression `fees * buyDevFee / buyTotalFees` may divide by zero. Its divisor has has estimated interval [0, 50]. --- ```solidity=1222 tokensForMarketing += fees * buyMarketingFee / buyTotalFees; ``` The expression `fees * buyMarketingFee / buyTotalFees` may divide by zero. Its divisor has has estimated interval [0, 50]. --- ```solidity=1277 uint256 liquidityTokens = contractBalance * tokensForLiquidity / totalTokensToSwap / 2; ``` The expression `contractBalance * tokensForLiquidity / totalTokensToSwap` may divide by zero. Its divisor has has estimated interval [0, pinf]. --- ```solidity=1286 uint256 ethForMarketing = ethBalance.mul(tokensForMarketing).div(totalTokensToSwap); ``` The expression `ethBalance.mul(tokensForMarketing).div(totalTokensToSwap)` may divide by zero. Its divisor has has estimated interval [0, pinf]. --- ```solidity=1287 uint256 ethForDev = ethBalance.mul(tokensForDev).div(totalTokensToSwap); ``` The expression `ethBalance.mul(tokensForDev).div(totalTokensToSwap)` may divide by zero. Its divisor has has estimated interval [0, pinf].

A sandwich attack might 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 (before the transaction being attacked) a transaction to purchase one of the assets and make profits by backrunning (after the transaction being attacked) 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: ```solidity=1243 uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens( tokenAmount, 0, // accept any amount of ETH path, address(this), block.timestamp ); ``` ```solidity=1256 uniswapV2Router.addLiquidityETH{value: ethAmount}( address(this), tokenAmount, 0, // slippage is unavoidable 0, // slippage is unavoidable deadAddress, block.timestamp ); ```

There are some redundant or unnecessary codes in the codebase. 1. Comparisons that are always true or always false may be incorrect or unnecessary. The check to ensure `_percent >= 0` will always be true for `uint256` values: ```solidity=1310 require(_percent <= 1000 && _percent >= 0, "Must set auto LP burn percent between 0% and 10%"); ``` 2. Duplicate check regarding `to != address(0)`. The `_transfer()` function already requires `to` to not be the zero address: ```solidity=1134 if ( from != owner() && to != owner() && to != address(0) && to != address(0xdead) && !swapping ) ``` 3. Unnecessary increment of zero: ```solidity=1170 if (block.number <= (launchedAt + 0) && to != uniswapV2Pair && to != address(0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D) ) { _blacklist[to] = false; } ```

State variables that never change should be declared as `constant` to save gas. ```solidity=881 uint256 public manualBurnFrequency = 30 minutes; ``` - `manualBurnFrequency` should be declared `constant`.

The low-level `call` function returns the status of the call as first variable in the returned tuple. The status of the `call` is not asserted to be `true`, which would treat the low-level call as a success even when it has reverted. ```solidity=1297 (success,) = address(devWallet).call{value: ethForDev}(""); ``` ```solidity=1305 (success,) = address(marketingWallet).call{value: address(this).balance}(""); ```