Findings
Issue 01
Description
approve is being called on every transaction which automatically contributes liquidity to the pool, on the same tokens and for the same spender (the router).
Recommendation
In order to reduce gas costs, approve could be called once (with max int).
Notes
The team has fixed the issue but it is preferable to increase the allowance automatically by checking it before the swap.
Issue 02
Description
_transfer calls the external function onFeeReceived, addLiquidityETH, and swapExactTokensForETHSupportingFeeOnTransferTokens .
Since this function can be called during _transfer, it may cause _transfer to fail unnecessarily.
Recommendation
Use try-catch when calling external functions in critical path flows.
Our recommendation is to Always make sure error cases are handled gracefully in critical functions such as _transfer.
Issue 03
Description
The token can become untradeable, should the owner set feeReceiver to a contract without the onFeeReceived function, causing the function to fail on every transfer.
Recommendation
Use try-catch to ensure the token is tradeable when there is an issue with feeReceiver.
Issue 04
Description
The fees and amounts to liquify can be simplified.
totalFee is used in multiplication and then division, which is redundant.
After removing totalFee from the calculation, totalBNBFee can also be removed as its a duplication of amountToSwap.
Considering the following changes, amountBNBLiquidity can be calculated in a simpler way:
Issue 05
Description
The owner can change the pair and router addresses. If they change either of them to a fake address the token will be untradeable.
Recommendation
Consider implementing a timelock mechanism to allow users to review the changes, or remove this feature altogether as backward-incompatible router updates are a very rare event. The pair address can be calculated on chain on the constructor by calling the function "createPair" or "getPair" please make sure you understand the difference between them.
Issue 06
Description: The owner can set the fees to any value they want, setting the fees to a value greater than 49% will make the token untradeable.
Recommendation: Set an upper limit to the set functions. In addition, make sure the total fees won't exceed a reasonable number.
Issue 07
Description
There is a lower limit to the set function but it isn't calculated correctly.
maxTxAmount = _tokenTotal.mul(percent).div(percentDecimals);, should be calculated as maxTxAmount = _tokenTotal.mul(percent).div(10**percentDecimals);
Issue 08
Description
At the moment swapExactTokensForETHSupportingFeeOnTransferTokens is called on every sell, this function sells tokens for BNB which can result in a price drop.
Consider limiting the price impact of such function.
At the moment, the amount of tokens that are being sold doesn't have an upper limit.
Recommendation
Consider adding a dynamic upper limit according to % of the token pair balance in order to control the price impact and to prevent the token from being untradeable due to slippage higher than 49%.
Second Revision
The team added a limit to the swap but the owner can change it and set it to any value they want (including 100% of the pair's balance).
Issue 09
Description
The contract assumes all the tokens were used for swapping to BNB or adding liquidity. however , add liquidity can result in not using all the fee tokens. Which will result in stuck tokens or BNB in the contract.
Recommendation
Consider checking the remaining token balance and assign the amount of fees according to the desired ration. In addition make sure you have a function to withdraw stuck ETH from the contract.
Issue 10
Description
There is a receive function in the contract, which means any address can send BNB to the contract.
Recommendation
In order to prevent the contract from receiving BNB from investors, which will result in a loss of funds, our recommendation is to only accept BNB from “whitelisted” addresses (e.g. the router address). The receive function will revert if the address is not whitelisted.
Issue 11
Description
The _excluded array can grow indefinitely. _transfer calls getCurrentSupply function which iterates over _excluded array. Iterating over an unbounded array can exceed the block gas limit which will make the token untradable.
Recommendation
A possible solution is to limit the number of excluded addresses. The issue described could be resolved by calling includeAccount which removes an element from the _excluded array.
Last updated