Sign Up for a Code Audit Today and Get 3 Months of Skynet for Free

In the contract `Petoverse` 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 set fees, auto rebase status and change fee receivers.
![](https://accelerator-tasks-prod.s3.amazonaws.com/d1fe75e447404de78f92b9011605543d/diagrams/centralization_Petoverse__owner.svg)

Centralization Risks in Petoverse.sol

The ERC20 [standard](https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#ERC20-decreaseAllowance-address-uint256-) assumes the following requirements for the `decreaseAllowance()` function:
- the spender cannot be the zero address.
- spender must have allowance for the caller of at least `subtractedValue`.

Missed Requirements in Function `decreaseAllowance()`

A state variable is shadowing another component defined in a parent contract.

Variable `_decimals` in `Petoverse` shadows the variable `_decimals` in `ERC20Detailed`;

Variable `_name` in `Petoverse` shadows the variable `_name` in `ERC20Detailed`;

Variable `_symbol` in `Petoverse` shadows the variable `_symbol` in `ERC20Detailed`.




Shadowing State Variable

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 `gameTreasuryReceiver` or `petoInsuranceFundReceiver`, they can make a recursive call back to the `transferFrom()`, repeating interactions that would have otherwise not run after the external call resolved the effects.

Potential Reentrancy Attack

In function `addLiquidity()`, the `IPancakeSwapRouter.swapExactTokensForETHSupportingFeeOnTransferTokens` function converts a half of the PETO to BNB. For every `addLiquidity()` function call, a small amount of BNB leftover in the contract. This is because the price of PETO drops after swapping the first half of PETO tokens into BNBs, and the other half of PETO tokens require less than the converted BNB to be paired with it when adding liquidity. The contract doesn't appear to provide a way to withdraw those BNB, and they will be locked in the contract forever.

Contract May Gain Non-withdrawable BNB

The linked functions are equivalent to the compiler-generated getter functions for the respective variables.

User-Defined Getters

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

Divide Before Multiply

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

Missing Zero Address Validation

The input parameter `_spender` in function `approve()` should be verified as non zero value to prevent being mistakenly assigned as address(0) in function.

Missing Zero Address Validation in `approve()` Function

The change of `owner` by function `transferOwnership()` overrides the previously set `owner` with the new one without guaranteeing the new `owner` is able to actuate transactions on-chain.

Pull-Over-Push Pattern

The contract is serving as the underlying entity to interact with third party Automated Market Maker and Anti Bot protocols. The scope of the audit treats 3rd party entities as black boxes and assume 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 `rebase()` function uses the magic number `15` to initialize the variable `epoch`.

Usage of Magic Number to Declare the `epoch`

The `rebase()` function uses the different magic numbers for `rebaseRate`.

Usage of Magic Number to `rebaseRate`

The contract works this way:
1. On `transfer()` 3% of the amount is taken as `liquidityFee` and stored on `autoLiquidityReceiver` via call to `takeFee()`
2. Every `48 hours`, the `addLiquidity()` is called due to next `transfer()`
3. During that all PETO added from `autoLiquidityReceiver` to `address(this)`
4. One half of PETO is converted into BNB via call to `IPancakeSwapRouter.swapExactTokensForETHSupportingFeeOnTransferTokens()`
5. Then `IPancakeSwapRouter.addLiquidityETH()` is called with half of PETO and the converted amount of BNB
6. No price Oracle is used, slippage is not controlled

This allows the attacker to steal part of `balanceOf(this)` via a flashloan attack.

Consider that a PETO loan can be obtained via any protocol that supports it, except the PancakePair interacted by `this`.

The scenario:
1. Let's assume PancakePair has liquidity of **100,000 PETO and 50 BNB** with a fair market price of 2000 PETO per 1 BNB.
2. Let's assume `this` contract has accumulated 100 PETO via taking `liquidityFee` on each transfer.
3. The attacker borrows 100,000 PETO and swaps it for 25 BNB. The pool price becomes 8000 PETO per 1 BNB. 4 times higher than expected.
5. The attacker waits after `_lastAddLiquidityTime` for 48 hours and sends a small amount of PETO via `transfer()` triggering the `addLiquidity()` call.
6. `this` swaps 50 PETO for ~0.00625 BNB via the imbalanced pool. If the pool was in the balanced state, this would get 0.025 BNB instead.
7. `this` calls `addLiquidity(50 PETO, 0.00625 BNB)` and gets 0.025% LP on `DEAD` balance. The pool state becomes 200,050 PETO and 25 BNB.
8. The attacker swaps back 25 BNB for PETO. The amount of BNB grows two times, the amount of PETO decreases two times. He/she gets about 100,025 PETO. The pool state becomes **100,025 PETO and 50 BNB**. Slightly higher than initially.
9. The attacker repays 100,000 PETO loans.
10. The attacker gets 25 PETO clear profit. In fact, that was stolen from the project.

However, obtaining a significant amount of PETO to perform the attack is not so easy. Taking into consideration the MAX total supply of PETO is 800,000,000, we set the Minor severity level for this finding.

Part of the Liquidity Fees Can Be Stolen via Flashloan Attack on `addLiquidity()`

`_allowedFragments[address(this)][address(router)]` is assigned explicitly, and as a result `emit` is missing.

`_allowedFragments` Should Be Approved

If `rebase()` is not called for more than a year, `_totalSupply` will be counted based on `rebaseRate = 211` during this time

Incorrect Сalculation of `_totalSupply`.

The `_basicTransfer()` function is missing emit when transferring to the `to` address.
The `takeFee()` function is missing emit when transferring to the `autoLiquidityReceiver` address.

Missing Emit Events on Transfer

The `manualSync()` function in the `Petoverse` contract is unnecessary. The `PancakeSwapPair.sync()` function can be used directly instead. 

Unnecessary `manualSync()` Function

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 could be declared as `constant` since these state variables are never modified.

Variables That Could Be Declared as `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`

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.

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

The aforementioned lines perform a low level call to execute transactions but the return value is not checked in either case.

Unchecked Value of low-level Call

A local variable `_owner` shadows the variable `_owner` in `Ownable`.

Shadowing Local Variable

The contract has 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 an 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 variables `swapEnabled`, `pairAddress` and `_decimals` are declared but the current code logic doesn't use these variables.

Unused Variables

The `setFee()` function uses the magic number `250` for a fee limit.

Usage of Magic Number to Declare the Fee Limit

Internal and private function names don't start with "_".

Declaration Naming Convention

`_totalFee` can be used instead of `totalFee`.

`_totalFee` Can Be Used Instead of `totalFee`.

The ERC20 [standard](https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#ERC20-increaseAllowance-address-uint256-) assumes the following requirement for the `increaseAllowance()` function:
- the spender cannot be the zero address.

Petoverse

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

32

0

32

0

0