$SHARBI
CertiK reviewed SHARBI ($SHARBI) on 26 May 2023: 36 findings, 0 critical; 1 marked fixed, 3 acknowledged, 32 open.
Report hosted on skynet.certik.com.
Trustblock does not review code.
Findings36
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 0 |
| High | 0 | 1 | 1 |
| Medium | 0 | 2 | 0 |
| Low | 32 | 0 | 0 |
Low`_transferFrom()` function should also exclude boOpen
The statement in the `_transferFrom()` function contains the following line to restrict transfers out from bots. ```solidity=662 require(!isBot[sender],"Bot detected"); ``` The sender should be blocked from sending out funds, but the bot receivers should also be restricted from receiving funds as well.
Low`multiTransfer()` may run out of gasOpen
The function `multiTransfer()` may run out of gas if `accounts.length` is too long. This because each iteration of the for loop will perform all the operations made inside `_transferFrom()` such as distribute dividends, executing `swapBack()`, executing `process()`. Specifically `process()` will also iterate on a while loop inside each for loop iteration.
Low`setSwapBackSettings()` execution could be lockedOpen
If `setSwapBackSettings()` is called passing `_amount = 0`, `swapThreshold` will be set accordingly. The function `setSwapBackSettings()` can never be called again to edit `swapThreshold` due to the check: ```solidity=965 require(swapThreshold > 0); ```
LowCatch Not Properly HandledOpen
Inside the linked `try ... catch` statements, the "catches" are empty.
LowDiscussion on `shouldSwapBack()` conditionOpen
The function `shouldSwapBack()` returns true only if several conditions are met together. Among them there is the following: ```solidity msg.sender != pair ``` This condition allows `shouldSwapBack()` to return true only if the transfer sender is not the pair, namely buy operations will not be allowed to swap back.
LowFunction `approve` of Contract `SHARBI_Token` DoeOpen
It is expected that calls of the form `approve(spender, amount)` fail if the address in `spender` is the zero address.
LowFunction Initializing StateOpen
State variables are inline initialized using either non-constant state variable or function calls that are not pure/constant. Since inline initialization occurs before constructor call, some non-constant state variables may not be initialized and non-constant functions may behave in an unexpected way. ```solidity=313 uint256 public minDistribution = 1 * (10**USDC.decimals()); ```
LowGas issues in process()Open
The function `process()` shows two issues regarding gas calculations: 1. It's not checked if the input `gas` is lower than the `gasleft()`. Indeed if the `gas` in input is greater than the remaining gas the function will revert at some point 2. Before the while loop ends the current value of `gasleft()` is stored inside the local variable `gasLeft`. However the value stored at an iteration x is used in the iteration x+1 in order to calculate the value of `gasUsed`. The problem is that when the gas left is saved during iteration x, the following operations also occur before the end of the iteration: ```solidity=406 currentIndex++; iterations++; ``` This means that the real gas left after the iteration x is lower than the value stored in `gasLeft`. Thus the variable `gasUsed` will be increased more quickly than it should be and the while loop will reach the exit condition sooner.
LowInconsistency between variable names and platformOpen
Inside the codebase at the linked locations different variables are using in the name the world `BNB` instead of `ETH` which is the current platform.
LowIneffective "anti-bot logic"Open
The anti-bot logic implemented in the code will label as bot all the addresses that interacted with the `pair` within the first 30 seconds after `enableTrading()` is called, causing `launchedAt` to be set. This logic will not catch all the bot activities that happen after the first 30 seconds. Usually, anti-bot logics work on monitoring the number of transactions made by the same addresses within the a reasonable time window instead of labelling as bots all the addresses that interacted within a certain time.
Show 10 more
LowIneffective limit on wallet balanceOpen
The initial value of `maxWalletAmount` is equal to `_totalSupply / 1000` which means `1_000_000_000 * 10**9`. This limit can be increased with `setMaxWalletAmount()`. Suppose that it is increased to `2_000_000_000 * 10**9` Then, if the limit will be lowered again to the initial value of `1_000_000_000 * 10**9`, some wallets may already have an amount of tokens greater than the new limit, causing it to be ineffective. **This finding severity is Informational since this causes no security issues: wallets with token amounts greater than the new limit are not blocked from transferring surplus tokens to other addresses.**
LowInefficient operations of `currentIndex`Open
The `currentIndex` is a global variable and frequently changing its value would cost lots of gas. Any additional waste of gas might significantly increase the risk of token transfer failures. In particular, inside `process()`, `currentIndex` is accessed and updated in each iteration of the while loop.
LowLack of input validations on `gas` parameter in `Open
In function `distributor.process()` there are no limits on the input parameter `gas` (or the variable `distributorGas` when run inside `SHARBI_Token._transfer()`) therefore it could potentially be set at an extremely low value and cause the execution of the `process()` function (and the distributing of dividends) to fail because of insufficient gas (especially when the `shareholder` array) since the function needs to loop through all shareholders to distribute dividends to eligible shareholders.
LowMissing bounds in `setDistributionCriteria()`Open
Inside the function `setDistributionCriteria()` there are no lower bounds or upper bounds for the value that can be assigned to `minPeriod` and `minDistribution`.
LowMissing checksOpen
Inside the codebase the following checks are missing: - In `setSwapBackSettings()` there is no input validation made on `_amount` - In `setIsDividendExempt()` it's not checked if the `holder` address in input is different from `address(router)`, `DEAD` and `ZERO` addresses as done in the constructor. - In `multiTransfer()` the boolean return value of each call to `_transferFrom()` inside the for loop is not checked
LowMissing Emit EventsOpen
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
LowMissing Error MessagesOpen
The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.
LowMissing Zero Address ValidationOpen
The following addresses should be checked before assignment or external call to make sure they are not zero addresses: - `_autoLiquidityReceiver` in `setFeeReceivers()` - `holder` in `setIsFeeExempt()` - `holders` in `setIsLimitExempt()` - `holder` in `setIsWalletExempt()`
LowPotential Risks on Approval/TransferFrom MethodsOpen
The function `approve()` could be used in an attack that allows a spender to transfer more tokens than the owner of the tokens ever wanted to allow the spender to transfer. Here is a possible attack scenario: Alice allows Bob to transfer N of Alice's tokens (N>0) by calling approve method on Token smart contract passing Bob's address and N as method arguments After some time, Alice decides to change from N to M (M>0) the number of Alice's tokens Bob is allowed to transfer, so she calls approve method again, this time passing Bob's address and M as method arguments Bob notices Alice's second transaction before it was mined and quickly sends another transaction that calls ·transferFrom· method to transfer N Alice's tokens somewhere If Bob's transaction will be executed before Alice's transaction, then Bob will successfully transfer N Alice's tokens and will gain the ability to transfer another M tokens Before Alice noticed that something went wrong, Bob calls ·transferFrom· method again, this time to transfer M Alice's tokens. So, Alice's attempt to change Bob's allowance from N to M (N>0 and M>0) made it possible for Bob to transfer N+M of Alice's tokens, while Alice never wanted to allow so many of her tokens to be transferred by Bob. ERC20 API: An Attack Vector on Approve/TransferFrom Methods: https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit#heading=h.m9fhqynw2xvt
LowPotential Sandwich AttacksOpen
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: - `router.swapExactTokensForETHSupportingFeeOnTransferTokens()` - `router.addLiquidityETH()` - `router.swapExactETHForTokensSupportingFeeOnTransferTokens()`
Show 10 more
LowPrivileges not Revoked or TransferredOpen
When `autoLiquidityReceiver` is updated through `setFeeReceivers()` the existing privileges owned by the old address are not revoked. Furthermore the same privileges are not granted to the new `autoLiquidityReceiver`
LowRedundant Code ComponentsOpen
The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.
LowRedundant setting of allowance to routerOpen
The allowance from the contract to the `router` contract has originally been set in the `constructor()`. ```solidity=584 _allowances[address(this)][address(router)] = _totalSupply; ``` Since there is no minting of tokens and the allowance has already been set to the max supply, there is no need to set the value again within the `swapBack()` function.
LowState Variable Should Be Declared ConstantOpen
State variables that never change should be declared as `constant` to save gas. ```solidity=296 IERC20Extended public USDC = IERC20Extended(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48); ``` - `USDC` should be declared `constant`. --- ```solidity=310 uint256 public dividendsPerShareAccuracyFactor = 10**36; ``` - `dividendsPerShareAccuracyFactor` should be declared `constant`. --- ```solidity=507 address public USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48; ``` - `USDC` should be declared `constant`. --- ```solidity=533 uint256 public snipingTime = 30 seconds; ``` - `snipingTime` should be declared `constant`.
LowThe deflation only happens whenever `sender` or `Open
As shown in the code snippet below, the current implementation only deducts when either the `sender` or the `receiver` is the `pair` address. ```solidity if ( isFeeExempt[sender] || isFeeExempt[recipient] || (sender != pair && recipient != pair) ) { amountReceived = amount; } else { uint256 feeAmount; if (sender == pair) { feeAmount = amount.mul(totalBuyFee).div(feeDenominator); amountReceived = amount.sub(feeAmount); takeFee(sender, feeAmount); setBuyAccFee(amount); } else { feeAmount = amount.mul(totalSellFee).div(feeDenominator); amountReceived = amount.sub(feeAmount); takeFee(sender, feeAmount); setSellAccFee(amount); } } ``` Normal transfers between users do not have any fees and do not act deflationary.
LowTypoOpen
Inside the linked error message `disable` is written instead of `disabled`
LowUnchecked ERC-20 `transfer()` CallOpen
The return value of the following `transfer()` call is not checked. ```solidity=429 USDC.transfer(shareholder, amount); ```
LowUnnecessary Use of SafeMathOpen
The `SafeMath` library is used unnecessarily. With Solidity compiler versions 0.8.0 or newer, arithmetic operations will automatically revert in case of integer overflow or underflow. ```solidity=15 library SafeMath { ``` - An implementation of `SafeMath` library is found. --- ```solidity=286 using SafeMath for uint256; ``` - `SafeMath` library is used for `uint256` type in `DividendDistributor` contract. ```solidity=357 totalShares = totalShares.sub(shares[shareholder].amount).add(amount); ``` - `SafeMath.sub` is called in `setShare` function of `DividendDistributor` contract. *Note: Only a sample of 2 `SafeMath` library usage in this contract (out of 15) are shown above.* --- ```solidity=500 using SafeMath for uint256; ``` - `SafeMath` library is used for `uint256` type in `SHARBI_Token` contract. ```solidity=650 _allowances[sender][msg.sender] = _allowances[sender][msg.sender] .sub(amount, "Insufficient Allowance"); ``` - `SafeMath.sub` is called in `transferFrom` function of `SHARBI_Token` contract. *Note: Only a sample of 2 `SafeMath` library usage in this contract (out of 39) are shown above.*
LowUnrestricted fee settingOpen
The setting of both buy fees and sell fees in the functions `setBuyFees()` and `setSellFees()` do not set requirements for the minimum and maximum values for the `_reflectionFee`, `_liquidityFee` nor `_feeDenominator` except for the requirement that `totalBuyFee <= feeDenominator.div(10)`. This would mean that these values could potentially result in overflow issues and `_reflectionFee`, `_liquidityFee` can also potentially be 0, which would mean that the $SHARBI token would cease to be a deflationary token.
LowUnused Return Values cause wrong value emitted inOpen
The return values `amountToken`,`amountETH` and `liquidity` returned from `router.addLiquidityETH()` are not stored or used. ```solidity=821 router.addLiquidityETH{value: amountBNBLiquidity}( address(this), amountToLiquify, 0, 0, autoLiquidityReceiver, block.timestamp ); ``` --- Furthermore the event `AutoLiquify` emits `amountBNBLiquidity` and `amountToLiquify` as the value added as liquidity instead of the real values `amountToken` and `amountETH`.
Show 2 more
LowUsage of `transfer` for sending EtherOpen
It is not recommended to use Solidity's `transfer()` function for transferring Ether, since some contracts may not be able to receive the funds. The `transfer()` function forwards only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=888 payable(owner()).transfer(amount); ``` - `SHARBI_Token.removeStuckEth` uses `transfer()`.
LowVariables That Could Be Declared as ImmutableOpen
The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.
Contracts in scope1
| Address | Chain | Source code |
|---|---|---|
| 0xf3a3023e6dede84ad88a656a3269f2a36e83c9a9 | Chain: Ethereum | Source code: Verified source |
Part of SHARBI’s security record
Security firm
CertiK1,044 projects reviewed · 1,375 reports · latest May 2025
See CertiK’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us