Read the findings of the 2022 Web3 Security Report

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

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

---


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

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

In the contract `ZenMaster` the role `devaddr` has authority over the functions shown in the diagram below.



![](https://accelerator-tasks-prod.s3.amazonaws.com/bcf5327959c244718483b0e07c5566a6/diagrams/centralization-ZenMaster-devaddr.svg)


---

In the contract `Sensi` the role `_owner` has authority over the functions shown as below:


- `updateMultiplier()`
- `add()`
- `set()`


Any compromise to the privileged accounts may allow the hacker to take advantage of this authority and update the sensitive settings and executive sensitive functions of the project.

Centralization Related Risks

Lacking `_moveDelegates()` invocation in the following functions will cause severe issues:

- In `transfer()` and `transferFrom()` of the `BEP20` contract, the voting power of the delegatee can't be moved from token sender to token recipient. 
- In `_burn()` and `_burnFrom()` of the `BEP20` contract, the voting power of delegatee can't be moved from token sender to `address(0)`. 

Voting Power Not Moved Along With `burn()`, `burnFrom()`, `transfer()` and `transferFrom()`

Any comparison should avoid using "now" or any other timestamp as any powerful miner can dominate the minding and thus manipulate the timestamp, which will eventually lead to vulnerability as deny delegation in certain situations.

Time Check Relies on Timestamp

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

Missing Emit Events

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

Improper Usage of `public` and `external` Type

The comment in L122, mentioned // XXX DO NOT add the same LP token more than once. Rewards will be messed up if you do.

The total amount of reward in function `updatePool()` will be incorrectly calculated if the same LP token is added into the pool more than once in function `add()`.

However, the code is not reflected in the comment behaviors as there isn’t any valid restriction on preventing this issue.

The current implementation is relying on the credibility of the owner to avoid repeatedly adding the same LP token to the pool, as the function will only be called by the owner.

`add()` Function Not Restricted

The ZenMaster contract operates as the main entry for interaction with staking users. The staking users deposit LP tokens into the pool and in return get a proportionate share of the pool’s rewards. Later on, the staking users can withdraw their own assets from the pool. In this procedure, `deposit()` and `withdraw()` are involved in transferring users’ assets into (or out of) the Omni Dex Finance protocol. When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged (and burned) transaction fee. As a result, this may not meet the assumption behind these low-level asset-transferring routines and will bring unexpected balance inconsistencies.

Incompatibility With Deflationary Tokens

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. 

In the function `emergencyWithdraw()`, if the `pool.lpToken.safeTransfer()` happened to be a malicious function that can cause the reentrancy, then the certain amount of token, `user.amount`, can be withdrawn multiple times. 

```solidity
function emergencyWithdraw(uint256 _pid) public {
    PoolInfo storage pool = poolInfo[_pid];
    UserInfo storage user = userInfo[_pid][msg.sender];
    if(_pid == 0) {
        xcharm.burn(msg.sender, user.amount);
    }
    pool.lpToken.safeTransfer(address(msg.sender), user.amount);
    emit EmergencyWithdraw(msg.sender, _pid, user.amount);
    user.amount = 0;
    user.rewardDebt = 0;
}
```

Although `pool.lpToken` can be checked by the owner when adding to the project by calling `add()`, it is still worthy mentioning this potential issue.

Potential Reentrancy Issue

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

There's no sanity check to validate if a pool is existing.

Recommended Explicit Pool Validity Checks

Omni Dex Finance

ZenMaster.sol

Sensei.sol

OmniDex Finance

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

10

0

8

1

1