<username>{{ maskedUsername }}</username><space /><text>just voted for</text><space /><projectButton />

<username>{{ maskedUsername }}</username><space /><text>just voted for</text><space /><projectLogo /><space /><projectName>{{ projectName }}</projectName>

Bug Bounty rewards are grouped by severity and the impact of the vulnerability. Examples and descriptions of the type of vulnerabilities that correspond to each severity level can be found below. The payout amount has a 5 tier structure with separate scalesfor website/apps and smart contracts.

Description of vulnerability and your proof of concept or validation steps

Name

Please input a title

Short description of the vulnerability and the affected asset.

The core team is situated in country(ies) that received low aggregated ratings in terms of international judicial cooperation, criminal justice system, anti-money laundering, and corruption. This contextual data suggests an increased difficulty and complexity in preventing and mitigating fraud.

The core team is situated in country(ies) that received medium aggregated ratings in terms of international judicial cooperation, criminal justice system, anti-money laundering, and corruption. This contextual data suggests a medium level of geographic accountability, thus reducing and mitigating the risk of fraud.

The core team is situated in country(ies) that received the best aggregated ratings in terms of international judicial cooperation, criminal justice system, anti-money laundering, and corruption. This contextual data suggests the highest level of geographic accountability, thus maximizing the risk reduction and mitigation.

Vulnerability Title

All **SHARBI** tokens are sent to the contract deployer when deploying the contract. This is a potential centralization risk as the deployer can distribute **SHARBI** tokens without the consensus of the community.

The detailed holder list can be found at Etherscan: https://etherscan.io/token/0xf3a3023e6dede84ad88a656a3269f2a36e83c9a9#balances 

Initial Token Distribution

In project `SHARBI`, multiple privileged roles are adopted to ensure the dynamic runtime updates and management of the project.

### DividendDistributor

In the contract `DividendDistributor` the role `token` has authority over the functions shown in the diagram and list below.
* setDistributionCriteria - Set distribution parameters
* setShare - Update shareholder balances
* process - Distribute dividends among all users
* deposit - Exchange ETH for USDC and update USDC dividend pool
  
Any compromise to the `token` account may allow the hacker to take advantage of this authority to control important settings, regulate distributions, and manage shareholder records.
![](https://accelerator-tasks-prod.s3.amazonaws.com/5ec593b6fa274e3ab51086c0b78fc620/diagrams/centralization_SHARBI_Token-DividendDistributor-token.svg)

### Ownable

In the contract `Ownable` the role `_owner` has authority over the functions shown in the diagram  list below.
* transferOwnership - Transfer ownership rights to another contract
* renounceOwnership - Give up ownership rights

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and control contract ownership rights.
![](https://accelerator-tasks-prod.s3.amazonaws.com/5ec593b6fa274e3ab51086c0b78fc620/diagrams/centralization_SHARBI_Token-Ownable-_owner.svg)

### SHARBI_Token

In the contract `SHARBI_Token` the role `_owner` has authority over the functions shown in the diagram and list below.
* setSwapBackSettings - Enable/disable swap backs and threshold amount
* setSellFees - Set sell liquidity and reflection fees
* setMaxWalletAmount - Set max amount that can be held by user in a single wallet
* setDistributionCriteria - Set minimum period and distribution amounts for auto processing distributions
* enableTrading - Enable swapping with liquidity pool
* removeStuckEth - Transfer leftover ETH to owner
* setMaxTxnAmount - Set max user transaction limits
* setBuyFees - Set buy liquidity and reflection fees
* setIsLimitExempt - Set transaction limit exemption
* multiTransfer - Make token transfers to multiple addresses
* setIsDividendExempt - Set exemption from dividend receipt
* setIsWalletExempt - Set exemption from wallet limit
* setIsFeeExempt - Set exemption from transaction fee deductions
* setDistributorSettings - Set gas amount for processing auto distributions
* setFeeReceivers - Set liquidity fee receiver addresses 
* removeBots - remove bot blacklist from accounts

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and call the above functions.
![](https://accelerator-tasks-prod.s3.amazonaws.com/5ec593b6fa274e3ab51086c0b78fc620/diagrams/centralization_SHARBI_Token-SHARBI_Token-_owner.svg)

Centralization Related Risks

When a transfer happens and the conditions in `shouldSwapBack()` are met, the function `swapBack()` is invoked. 
However, `swapBack()` uses `_liquidityFeeCount` and `_reflectionFeeCount`. Their initial value is zero by default and is set to zero again after `swapBack()` completes its execution. Their value is changed only in `setBuyAccFee()` and `setSellAccFee()` after transfers to/from the pair. 

```solidity=791
        uint256 amountToLiquify = swapThreshold
            .mul(_liquidityFeeCount)
            .div(totalFee)
            .div(2);
```

In a certain scenario, this logic will cause `swapBack()` to revert:
1. Suppose that `swapBack()` is invoked before any transfer occurs to/from the pair or that two `swapBack()` executions occur after some time, but in the meanwhile no transfer is happened to/from the pair
2. In both the scenarios described in the previous point, `_liquidityFeeCount` and `_reflectionFeeCount` are zero
3. The current execution of `swapBack()` will calculate a value for `totalFee` equal to zero at line 789
4. At line 793 when calculating `amountToLiquify` a division by zero occurs, and function revert

Another situation that could occur would be small amounts being transferred and used in the `setBuyAccFee()` and `setSellAccFee()` functions.  The amounts that might be truncated by division by the `feeDenominator` and would not update the `_liquidityFeeCount` and `_reflectionFeeCount` values.

```solidity=764
    function setBuyAccFee(uint256 _amount) internal {
        _liquidityFeeCount += _amount.mul(_liquidityBuyFee).div(feeDenominator);
        _reflectionFeeCount += _amount.mul(_reflectionBuyFee).div(
            feeDenominator
        );
    }

    function setSellAccFee(uint256 _amount) internal {
        _liquidityFeeCount += _amount.mul(_liquiditySellFee).div(
            feeDenominator
        );
        _reflectionFeeCount += _amount.mul(_reflectionSellFee).div(
            feeDenominator
        );
    }
```

Possible Denial of Service after `swapBack()` is run

The `addLiquidityETH` function calls the `swapBack` function with the `to` address specified as `autoLiquidityReceiver` for acquiring the generated LP tokens from the pool. As a result, over time the `autoLiquidityReceiver` address will accumulate a significant portion of LP tokens. If the `autoLiquidityReceiver` is an EOA (Externally Owned Account), the mishandling of its private key can have devastating consequences for the project.
```solidity
            router.addLiquidityETH{value: amountBNBLiquidity}(
                address(this),
                amountToLiquify,
                0,
                0,
                autoLiquidityReceiver,
                block.timestamp
            );
```

Generated LP tokens are transferred to the `autoLiquidityReceiver` When adding liquidity

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.

Gas issues in process()

The return value of the following `transfer()` call is not checked.

```solidity=429
            USDC.transfer(shareholder, amount);
```



Unchecked ERC-20 `transfer()` Call

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`.


Unused Return Values cause wrong value emitted in AutoLiquify

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()`.


Usage of `transfer` for sending Ether

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()`


Missing Zero Address Validation

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`

Privileges not Revoked or Transferred

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);
```


`setSwapBackSettings()` execution could be locked forever

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()`

Potential Sandwich Attacks

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.

Lack of input validations on `gas` parameter in `distributor.process()`

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.

`_transferFrom()` function should also exclude bots that are receivers

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.

Unrestricted fee setting

Inside the function `setDistributionCriteria()` there are no lower bounds or upper bounds for the value that can be assigned to `minPeriod` and `minDistribution`.

Missing bounds in `setDistributionCriteria()`

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

Missing checks

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.


Ineffective "anti-bot logic"

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.

`multiTransfer()` may run out of gas

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

Potential Risks on Approval/TransferFrom Methods

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.*



Unnecessary Use of SafeMath

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`.



State Variable Should Be Declared Constant

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.

Variables That Could Be Declared as Immutable

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.

Inefficient operations of `currentIndex`

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.

Redundant setting of allowance to router

It is expected that calls of the form `approve(spender, amount)` fail if the address in `spender` is the zero address.

Function `approve` of Contract `SHARBI_Token` Does Not Prevent Approvals for the Zero Address

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

Missing Emit Events

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.

Missing Error Messages

The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.

Redundant Code Components

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());
```



Function Initializing State

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.

Inconsistency between variable names and platform

Inside the linked `try ... catch` statements, the "catches" are empty.

Catch Not Properly Handled

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.

The deflation only happens whenever `sender` or `recipient` is `pair` address.

Inside the linked error message `disable` is written instead of `disabled`

Typo

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.

Discussion on `shouldSwapBack()` condition

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.**

Ineffective limit on wallet balance

$SHARBI

SHARBI_Token.sol

Our cryptocurrency is multichain and owned by the community. It is built on both Arbitrum and ETH, and we call it Sharbi. This token rewards its holders with USDC, and we aspire to create a decentralized WEB3 community that continues to provide USDC rewards to its members. We see ourselves as a meme community token 2.0 that can accomplish great things through the strength of our community. Remember to never underestimate the potential of a dedicated group of individuals working together towards a common goal.

Leaderboards

Due Diligence Tools

Resources

KYC Status

Bug Bounty Program