Findings
Issue 01
Description
addLiquidity calls the external function addLiquidityETH . 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 02
Description
swapTokensForEth calls external function swapExactTokensForETHSupportingFeeOnTransferTokens , and is called during _transfer, which can cause the same issues as specified in issue 01.
Recommendation
Same as Issue 01.
Issue 03
Description
approve is being called every transaction on the same tokens and for the same spender (the router).
Recommendation
In order to reduce gas cost, approve could be called once (with max int), and then check if it is needed again using allowance.
Second revision note
There is no way for the owner to increase allowance. Consider making sure the router has a sufficient allowance before swapping and adding liquidity.
Issue 04
Description
The recipient of the newly created LP tokens is the owner of the contract. The newly created LP tokens are unlocked.
Recommendation
Our recommendation is to change the recipient of the newly created LP tokens to the contract in order to ensure that the LP tokens are locked or to simply lock the tokens in the contract for a certain time frame.
The owner can withdraw any token from the contact including the native token which is collected for fees and the LP tokens which are sent to the token contract.
Issue 05
Description
The _excluded array can grow indefinitely. _transfer calls getCurrentSupply which iterates over the _excluded array. Iterating over an unbounded array can exceed the block gas limit which will make the token untradeable.
Recommendation
One possible solution is to limit the number of excluded addresses. We encourage the owner of the project and the community to frequently monitor the number of excluded addresses. The issue described could be recovered by calling includeInReward which removes an element from the _excluded array.
Issue 06
Description: The code is vulnerable to the SafeMoon bug - excluding an address from the fee and then later including it back will cause the address to receive all RFI rewards for the time it was excluded (at the expense of other holders).
In includeInReward, _rOwned is not updated. _rOwned should be updated and be calculated according to the current rate.
Recommendation:
Properly calculate the _rOwned of the included address in includeInReward based on its _tOwned amount
Issue 07
Description
SwapAndLiquify takes place only on sell transactions. There is a chance that the contract will accrue a significant amount of tokens. The consequence is a pretty significant price drop because of the large number of tokens that would enter the pool. The team should be aware of this issue, and monitor the lower limit for selling.
Recommendation
Consider having an upper limit to the amount of tokens that can be traded in a swap. In addition, consider setting it to a percentage of the pair's balance.
The issue was partially resolved by the team, the team added an upper limit for the internal swap but it's a constant number which the team should always monitor and make sure the price impact resulting by the internal swap doesn't exceed 49% slippage. Our recommendation, is to take the decentralized approach and make sure the price impact does not exceed a certain percentage of the pair balance. The team can recover from the issue by decreasing the amount of tokens that are sold by the contact manually.
Issue 08
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 ETH from “whitelisted” addresses (e.g. the router address). The receive function will revert if the address is not whitelisted.
There is a way for the owner to withdraw mistakenly sent BNBs to the contract.
Issue 09
Description
The owner of the contract can set the liquidity fee, tax fee and burn fee by calling setTaxFeePercent, setBurnFeePercent, and setLiquidityFeePercent. If the owner sets any of the fees to 100% the token will be untradeable.
Recommendation
Consider adding an upper limit to the set functions. In addition, make sure the total collected fees do not exceed a reasonable value.
Issue 10
Description
The owner can set the upper trade limit to any value they want. If they set it to 0, the token will be untradeable.
Recommendation
Set the max tx limit to be a percentage of the total supply. Moreover, add a lower limit to prevent it from being 0.
Issue 11
Description
An owner can gain ownership of the contract even if he calls renounceOwnership function.
This can be achieved by performing the following steps:
The owner of the contract can call lock() to lock the contract (the lock function saves the previous owner into a variable)
After the locking period has passed the owner of the contract can call unlock() and regain the ownership.
The owner of the contract can then call the renounceOwnership function. Now, the contract allegedly has no owner (users can verify it by looking for the renounceOwnership transaction and making sure that the owner is set to the zero address)
Recommendation
Remove these functions if not needed.
Issue 12
Description
If the owner sets the sale limit to 0, the token will be untradeable.
Recommendation
Consider if this function is needed, as they have solid group bot protection.
Last updated