Findings

Issue 01

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:

Our recommendation Is to remove the lock and unlock function if not needed

Issue 02

Description

The swap transaction will be executed immediately, no need to set time-out. 

 function swapETHForTokens(uint256 amount) private {
        // generate the uniswap pair path of token -> weth
        address[] memory path = new address[](2);
        path[0] = uniswapV2Router.WETH();
        path[1] = address(this);

      // make the swap
        uniswapV2Router.swapExactETHForTokensSupportingFeeOnTransferTokens{value: amount}(
            0, // accept any amount of Tokens
            path,
            deadAddress, // Burn address
            block.timestamp.add(300)
        );
        
        emit SwapETHForTokens(amount, path);
    }

Issue 03

approve is being called every transaction 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), and then check if it is needed again using allowance.

Issue 04

Description

_transfer may call internally to swapExactTokensForETHSupportingFeeOnTransferTokens ,addLiquidityETHand swapExactETHForTokensSupportingFeeOnTransferTokens Since this function can be called during _transfer, it may cause _transfer to fail unnecessarily.

Recommendation

_transfer should always work, and shouldn't fail if swapExactTokensForETHSupportingFeeOnTransferTokens or addLiquidityETH or any other router related fails in order to make sure the token will always be tradable.

Issue 05 

receive() external payable {}

Description

There is a receive function in the contract, which means any address can send BNB to the contract. The problem is that there is no way to recover BNB that were mistakenly sent 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 (for example the router address should be whitelisted). The receive function will revert if the address is not whitelisted.

Issue 06

Description

The owner of the contract can set the liquidity fee and tax fee by calling setLiquidityFeePercent and setTaxFeePercent to any value he desires. If the owner sets the fee to 100% the token will be untradeable.

Recommendation

Consider adding an upper limit to the set functions.

Issue 07

Description

The owner of the contract can make the token untradable by calling setMaxTxAmount with 0.

Recommendation

Consider preventing it in the code by adding a require statement in the set function.

Issue 08

Description

This function is taking fees from the transfer amount. When taking fee from the transfe amount it is best practice to emit events, so investors would know that fees were deducted.

Recommendation

Add Transfer event when taking fees.

Issue 09

Description

set functions don't emit events.

Recommendation

Consider emitting events when changing the state of the contract.

Issue 10

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 _rOwned of the included address in includeInReward based on its _tOwned amount.

PreviousOwner CapabilitiesNextSummary

Last updated