Read the findings of the 2022 Web3 Security Report

In the contract `Octro` the role `_owner` has authority over the functions shown in the diagram below.

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and for example:
1. Transfer ownership of the contract to an address he/she controls
2. From there, the hacker has the power to change the following addresses that receive tokens to addresses he/she controls:
   - `_autoLiquidityReceiver`
   - `_treasuryReceiver`
   - `_octroInsuranceFundReceiver`
   - `_furnace` 
3. Finally the hacker can renounce ownership of the contract, which would then make it impossible to change these addresses again.


![](https://accelerator-tasks-prod.s3.amazonaws.com/5aa98ea1f82c4e21a09653a33a28ceeb/diagrams/centralization-Octro-_owner.svg)

![](https://accelerator-tasks-prod.s3.amazonaws.com/5aa98ea1f82c4e21a09653a33a28ceeb/diagrams/centralization-Ownable-_owner.svg)

Centralization Risk in `Octro.sol`

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects.

Potential Reentrancy Attack

All of the `OCTRO` tokens are sent to the `treasuryReceiver` when deploying the contract. This could be a centralization risk as the owner of the `treasuryReceiver` address can distribute `OCTRO` tokens without obtaining the consensus of the community.

Initial Token Distribution

The return values of function  `addLiquidityETH` are not properly handled.

Return Value Not Handled

Addresses should be checked before assignment or external call to make sure they are not zero addresses.

Missing Zero Address Validation

The **for** loop below could be changed to arithmetical operation and stored using a local variable.

```solidity
  for (uint256 i = 0; i < times; i++) {
    _totalSupply = _totalSupply
    .mul((10**RATE_DECIMALS).add(rebaseRate))
    .div(10**RATE_DECIMALS);
  }
```

Unnecessary `for` Loop 

A sandwich attack might 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 (before the transaction being attacked) a transaction to purchase one of the assets and make profits by backrunning (after the transaction being attacked) 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:

- `addLiquidity()`
- `swapBack()`
- `addLiquidity()`

Potential Sandwich Attacks

The contract is serving as the underlying entity to interact with third-party protocols. The scope of the audit treats 3rd party entities as black boxes and assumes their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, etc.

Third Party Dependencies

The function `setWhitelist()` flags an address as being exempt from paying fees, however, there is no way to remove this exemption once an address is flagged.

`_isFeeExempt[_addr]` Cannot Be Changed Once Set to True

The contract has an unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to differing compiler version numbers. This can lead to ambiguity when debugging as compiler-specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.

Unlocked Compiler Version

The imported library `SafeMathInt` remains unused throughout the codebase.

Unused Imported Library

The `SafeMath` library and `Ownable` contract imported are outdated. Both are version 2, but should be version 3.

Outdated Libraries

The error message in the function `setBotBlacklist()` is misspelled.
```solidity
  require(isContract(_botAddress), "only contract address, not allowed exteranlly owned account");
```
The error message should instead be "only contract address, not allowed externally owned account".

Misspelled Error Message

The function `approve()` has no check to ensure the `spender` address is not the 0 address.

Missing Input Validation

State variables that never change should be declared as `constant` to save gas.

State Variable Should Be Declared Constant

Performing integer division before multiplication truncates the low bits, losing the precision of calculation.

Divide Before Multiply

`success` is written twice, but not used in-between.

```solidity
        (bool success, ) = payable(treasuryReceiver).call{
            value: amountETHToTreasuryAndSIF.mul(treasuryFee).div(
                treasuryFee.add(octroInsuranceFundFee)
            ),
            gas: 30000
        }("");
```

```solidity
        (success, ) = payable(octroInsuranceFundReceiver).call{
            value: amountETHToTreasuryAndSIF.mul(octroInsuranceFundFee).div(
                treasuryFee.add(octroInsuranceFundFee)
            ),
            gas: 30000
        }("");
```



Write After Write

State variables are inline initialized using either non-constant state variables or function calls that are not pure/constant. Since inline initialization occurs before the constructor call, some non-constant state variables may not be initialized and non-constant functions may behave in an unexpected way.

Function Initializing State

The functions which are never called internally within the contract should have external visibility for gas optimization.

Function Should Be Declared `external`

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

In the following block of code, the last two **else if** statements can never be reached.

`((15 * 365 days) / 10)` and `(7 * 365 days)` are both values greater than 365 days, however, the code will never reach these due to the second check being `deltaTimeFromInit >= (365 days)`


```solidity
if (deltaTimeFromInit < (365 days)) {
            rebaseRate = 2394;
        } else if (deltaTimeFromInit >= (365 days)) {
            rebaseRate = 211;
        } else if (deltaTimeFromInit >= ((15 * 365 days) / 10)) {
            rebaseRate = 14;
        } else if (deltaTimeFromInit >= (7 * 365 days)) {
            rebaseRate = 2;
        }
```

Unreachable Code 

The variable `swapEnabled` is initialized, but never used.

Octro Finance

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

24

0

23

0

1