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

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 different compiler versions. 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

ESG Token

ESG-ERC20.sol

In the contract `Esgtroller` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/Ecoptroller-admin.svg)

The role `borrowCapGuardian` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `borrowCapGuardian` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/Ecoptroller-borrowCapGuardian.svg)

The role `pauseGuardian` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `pauseGuardian` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/Ecoptroller-pauseGuardian.svg)

Centralization Risk in Esgtroller.sol

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.

All the files are affected.

In the contract `CDaiDelegate` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/CDaiDelegate-admin.svg)

Centralization Risk in CDaiDelegate.sol

In the contract `CErc20Delegator` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/CErc20Delegator-admin.svg)

Centralization Risk in CErc20Delegator.sol

In the contract `CErc20Delegate` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/CErc20Delegate-admin.svg)

Centralization Risk in CErc20Delegate.sol

Typo in Comments

In the contract `CErc20` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/CErc20-admin.svg)

Centralization Risk in CErc20.sol

Since the compiler 0.6.2 is used the "fallback function" should be declared as:
```solidity
  fallback() external payable {}
```

See [Solidity 0.6.0 breaking changes](https://docs.soliditylang.org/en/v0.7.4/060-breaking-changes.html#semantic-and-syntactic-changes)

Fallback Function Should Have Name 'fallback'

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions. `EsgChainlinkOracle.getUnderlyingPrice()` is declared as `public`, but `PriceOracle.getUnderlyingPrice()` is `external`.

Improper Usage of `public` and `external` Type

String literals "eBNB" and "eESG" are never mentioned except `EsgChainlinkOracle` and are not declared as constants.

Magic Strings `eBNB` and `eESG`

The linked statements do not affect the functionality of the codebase. `10**0` is equal to `1`, so there is no need to handle `decimalDelta == 0` situation specifically.

Redundant Statements in `EsgChainlinkOracle`

```solidity=44
        uint decimalDelta = uint(18).sub(uint(token.decimals()));
```
In case `token.decimals()` is bigger than 18 for some reason, the function will revert.

Also, the price decimals manipulation is unclear. 
1. `feed.latestAnswer()` will return ~1e8 for USDT
2. `getChainlinkPrice(USDTfeed)` will return ~1e18
3. `USDT.decimals()` will return 6
4. `getPrice(eUSDT)` will return ~`1e18 * 10 ** (18 - 6) = 1e30`
5. `getUnderlyingPrice(eUSDT)` will also return ~1e30

That result seems unexpected.

For "eESG" the "direct price" is returned directly from `getUnderlyingPrice()`. But from `getPrice()` the "direct price" is returned in the normalized form.

`getPrice()` Doesn't Work with `token.decimals()` Bigger than 18

After [EIP-1884](https://eips.ethereum.org/EIPS/eip-1884) was included in the Istanbul hard fork, it is not recommended to use `.transfer()` or `.send()` for transferring ether as these functions have a hard-coded value for gas costs making them obsolete as they are forwarding a fixed amount of gas, specifically `2300`. This can cause issues in case the linked statements are meant to be able to transfer funds to other contracts instead of EOAs.

Usage of `transfer()` for sending Ether

`ETokenInterfaces.sol` is imported but the filename is `CTokenInterfaces.sol`. The contract can't be compiled.

`ETokenInterfaces.sol` Is Imported instead of `CTokenInterfaces.sol`

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

`initialize()` function is not protected and can be called by anyone.

`Esgtroller.initialize()` Can Be Called by Anyone

`getEsgAddress()` returns the address of ESG token deployed on BSC chain. However, `totalSupply()` differs from the one specified in ESG.sol, ESG-ERC20.sol in the audit scope.  The voting power and `quorumVotes` values depend on the `totalSupply()`. 

`Esgtroller` implementation supposes setting `esg` address via `initialize()`.

ESG-ERC20.sol is supposed to be deployed on Ethereum, however, `JumpRateModel` relies on `blocksPerYear` for the BSC chain.

`getEsgAddress()` Is Redundant

Wrong Comments

There is a typo in variable name `sreTokensNew`. Expected `srcTokensNew`.

Typo in Variable Name `sreTokensNew`

In the contract `EToken` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/EToken-admin.svg)

Centralization Risk in EToken.sol

The `_reduceReservesFresh()` function comment states that the function transfer funds to admin. However, in fact it transfers to a hardcoded address `0x8AE905BdC3dA74DC8308cB15221814595eD73362`.

Hardcoded Address Is Used in `_reduceReservesFresh()`

In the contract `GovernorBravoDelegate` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/GovernorBravoDelegate-admin.svg)

Centralization Risk in GovernorBravoDelegate.sol

The malefactor can create malicious proposals. The project can be overtaken if participants are not voting actively.

1. The attacker can borrow 0.1% of ESG tokens (100 000 ESG, maximum possible `proposalThreshold`).
2. In the next block the call to `propose()` can be made with `Timelock.setPendingAdmin(attacker)` transaction.
3. Right before the `votingDelay` expires the attacker can borrow 0.3% or more of ESG tokens.
4. In the next block all except 0.1% of ESG tokens can be repayed. The attacker still has the voting power bigger than `quorumVotes` (400 000 ESG).
5. The malicious proposal can't be canceled during the pending or voting period.
6. Since no actual voting happens other users are not motivated to spend gas to vote against the proposal. Also, their snapshotted ESG amount can be insignificant at the moment of proposal creation. Also, it is **not known what is the sufficient votes power required** to reliably defeat the proposal.
7. After 1-14 days in the last block with number `proposal.endBlock` the attacker calls `castVote()` supporting her malicious proposal. All the votes snapshotted in step 3 can be used.
8. In the next block the attacker `queue()`s her succeeded proposal with `timelock.delay()` (2-30 days).
9. After this point the execution of malicious transaction can't be prevented.
10. The attacker `execute()`s the malicious transaction, gains control over Timelock and all the contracts controlled by it.

The Control over the Timelock Can Be Taken via Voting Manipulation

```solidity=18
    uint public constant MIN_VOTING_PERIOD = 5760; // About 24 hours 

    /// @notice The max setable voting period
    uint public constant MAX_VOTING_PERIOD = 80640; // About 2 weeks

    /// @notice The min setable voting delay
    uint public constant MIN_VOTING_DELAY = 1;

    /// @notice The max setable voting delay
    uint public constant MAX_VOTING_DELAY = 40320; // About 1 week
```
These constants assume a block is created once per 15 seconds, like in Ethereum. However, the project is going to be deployed on BSC with 5 times faster block production.

`VOTING_PERIOD` Constants Are Five Times Lower than Expected

Literals with many digits are difficult to read and review.

Too Many Digits

`propose()` is supposed to be called after the contract is initiated via `_initiate()`. However, this is not ensured.

`propose()` Can Be Called before `_initiate()`

```solidity=203
        } else if (proposal.forVotes <= proposal.againstVotes || proposal.forVotes < quorumVotes) {
            return ProposalState.Defeated;
        } else if (proposal.forVotes >= proposal.againstVotes && proposal.forVotes >= 500000e18 && proposal.eta == 0) {
            return ProposalState.Succeeded;
        } else if (proposal.executed) {
            return ProposalState.Executed;
        } else if (block.timestamp >= add256(proposal.eta, timelock.GRACE_PERIOD())) {
            return ProposalState.Expired;
        } else {
            return ProposalState.Queued;
```
In case `forVotes <= againstVoves` or quorum was not achieved, the proposal is defeated. However, to succeed the proposal requires `500_000e18` for unknown reason. The number doesn't depend on the `totalSupply()` of the voting token.

In case `quorumVotes <= proposal.forVotes < 500000e18` the proposal becomes `Queued` and can get `execute()`d immediately without waiting for `timelock.delay()` to expire.

Magic Number Is Used as `quorumVotes`

In the contract `GovernorBravoDelegator` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/GovernorBravoDelegator-admin.svg)

Centralization Risk in GovernorBravoDelegator.sol

```solidity=9
    function quorumVotes() public pure returns (uint) { return 400000e18; } // 400,000 = 4% of ESG

    /// @notice The number of votes required in order for a voter to become a proposer
    function proposalThreshold() public pure returns (uint) { return 100000e18; } // 100,000 = 1% of ESG
```
ESG total supply is 100 million. So, 400,000 ESG is 0.4%. `PROPOSAL_THRESHOLD` values are also affected.

`quorumVotes()` and `PROPOSAL_THRESHOLD` Are 10 Times Lower than Expected

```solidity=108
    function _acceptAdmin() public returns (uint) {
        // Check caller is pendingAdmin and pendingAdmin ≠ address(0)
        if (msg.sender != pendingAdmin || msg.sender == address(0)) {
```
The comment states that `pendingAdmin` should not be zero. However, the code checks that `msg.sender` is not zero instead, which is always true.

The function can't be executed with zero `pendingAdmin` anyway.

Wrong `pendingAdmin` Check in `_acceptAdmin()`

`owned.sol` was not audited, the code quality was not verified. The `owned` functionality is not used in the `JumpRateModel`.

`owned.sol` Is Out of Audit Scope

`esgtrollerImplementation` and other state variables were inserted directly in the middle of the existing storage structure. Upgradable contracts should add new state variables to the end.

State Variables Can't Be Reordered for Upgradable Contract

The functions `sqrt()`, `sq()` and `pwr()` are not used by the code and are not part of SafeMath library.

Unused Math Functions

`setUnderlyingPrice()` and `setDirectPrice()` can be called by everyone.

Functions in `SimplePriceOracle` Are Unprotected

Error message in constructor contains `Timelock::setDelay` instead of `Timelock::constructor`.

Wrong Error Message `Timelock::setDelay`

```solidity=34
    function() external payable { }
```
Timelock contract can accept ether, however, there is no reason for that. All the ether can be supplied via `executeTransaction()`. Providing ether beforehand can lead to spending it by another queued transaction.

No Reason to Accept Ether

The functions are declared as `public`. The functions that are never called internally within the contract should have `external` visibility. The `memory` arguments should be declared as `calldata` if they are not changed in the function.

Function Visibility Optimization in Timelock Contract

In the contract `Timelock` the role `admin` has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `admin` may allow the hacker to take advantage of this.

![](https://accelerator-tasks-prod.s3.amazonaws.com/2ad9f2887b6a477dad1fa62485f52b95/Timelock-admin.svg)

Centralization Risk in Timelock.sol

```solidity=15
    uint public constant GRACE_PERIOD = 86400*14;
    uint public constant MINIMUM_DELAY = 86400*2;
    uint public constant MAXIMUM_DELAY = 86400*30;
```
Magic numbers can be replaced with time units suffixes (days, weeks).

Time Units Can Be Used

`setPendingAdmin()` can be called directly by `admin` without any delay. Taking into account the `admin` priviledges this practically makes Timelock less useful.

ESG Financial

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

1

0

1

0

0