Epoch Island - vEPOCH Token

Quantstamp3 Nov 2023Files named in the report

Quantstamp reviewed Epoch Island (Epoch Island - vEPOCH Token) on 3 Nov 2023: 18 findings, 0 critical; 4 marked fixed, 12 acknowledged, 2 open.

No open critical findings

Open the report Open Epoch Island’s security record

Report hosted on certificate.quantstamp.com.

Trustblock does not review code.

Findings18

Statuses are the security firm’s own, from its report.

Severity	Open	Acknowledged	Fixed
Critical	0	0	0
High	0	0	1
Medium	0	1	1
Low	2	11	2

MediumERC20 Token Transfer May Silently FailAcknowledged

Whenever a `rewardToken` or `depositToken` is transferred, they may fail without reverting. Depending on the context, this may lead to inconsistencies, users losing funds, or the contract losing funds. The reason is that calls to `transfer()` and `transferFrom()` are not checked for returning true. While many ERC20s will revert upon transfer failure, some ERC20s will return false in adherence to the ERC20 specification. If `rewardToken` or `depositToken` will return false on failure the transfer will silently fail. The return value is not checked in the following places: 1. `Vepoch.sol#L78`. 1. `Vepoch.sol#L108`. 1. `Vepoch.sol#L130`. 1. `Vepoch.sol#L146`. 1. `Vepoch.sol#L192`. 1. `Vepoch.sol#L210`. 1. `Vepoch.sol#L215`. 1. `Vepoch.sol#L234`. 1. `Vepoch.sol#L252`.

Low`deposits` Index Will Start at 1Acknowledged

The `deposits` mapping will begin at index one upon first `deposit()` call, leaving the zero'th index empty. This is because `depositCount` is incremented before setting the next `deposits` value. Particularly, if `deposits` is changed to an array for gas efficiency reasons, it is recommended to commence at index zero.

Low`owner` May Prevent Any Further DepositsAcknowledged

The `owner` may (accidentally) prevent further user deposits by setting `maxDepositDuration < 60`. This would lead to `deposit()` always reverting as `_duration > 59` and `_duration <= maxDepositDuration` cannot hold true at the same time for `maxDepositDuration < 60`. The root issue is that `setMaxDepositDuration()` does not enforce a lower bound on `maxDepositDuration`. Furthermore, the `owner` can (accidentally) set a too-low `maxDepositDuration` and lock that value forever by calling `setMaxDepositDurationLocked()`. This would lock out any future calls to `setMaxDepositDuration()` to undo such a setting

LowApplication Monitoring Can Be Improved by Emitting More EventsAcknowledged

In order to validate the proper deployment and initialization of the contracts, it is a good practice to emit events. Also, any important state transition can be logged, which is beneficial for monitoring the contract, and also tracking eventual bugs, or hacks. Below we present a non-exhaustive list of events that could be emitted to improve the application management: 1. `Vepoch.addRewardTokens()`: `rewardIndex`. 1. `Vepoch._updateRewards()`: `earned[]` and `rewardIndexOf[]`. 1. `Vepoch.setAuthorisedLocked()`: `authorisedLocked`. 1. `Vepoch.setMaxDepositDurationLocked()`: `maxDepositDurationLocked`. 1. `Vepoch.setForfeitRedirectionAddressLocked()`: `forfeitRedirectionAddressLocked`. 1. `Vepoch.setForfeitRedirection()`: `forfeitRedirectionAddress`.

LowAuthorised Addresses Can Always Transfer Once Control Is Given UpAcknowledged

Once the `owner` gives up control over authorising addresses by calling `setAuthorisedLocked()`, all authorised addresses will forever be able to transfer vEPOCH tokens. While the `owner` can "unauthorize" addresses, they may not do so for all intended addresses before giving up control.

LowCritical Role Transfer Not Following Two-Step PatternAcknowledged

The owner of the contracts can call `transferOwnership()` to transfer the ownership to a new address. If an uncontrollable address is accidentally provided as the new owner address then the contract will no longer have an active owner, and functions with the `onlyOwner` modifier can no longer be executed.

LowLack of Clear Reward IncentiveAcknowledged

The reward mechanism is supposed to be an incentive for users to lock their `depositToken`. However, their received rewards depends on the amount of rewards provided through `addRewardTokens()` during their lock period. This uncertainty may not pose a stable incentive for users to deposit and lock their tokens.

LowLogic Missing in `transfer()` and `transferFrom()` of the veTokenAcknowledged

Although this issue is only for authorized users, when veToken is transferred, the `rewardStakingPower`, `rewardIndexOf` and the deposits are not changed accordingly. This could cause the inability of authorized users in `withdraw()` and `withdrawForfeit()`. This would also lead to an incorrect calculation of rewards.

LowMissing Input ValidationAcknowledged

It is important to validate inputs, even if they only come from trusted addresses, to avoid human error. The following functions do not have a proper validation of input parameters: 1. `Vepoch.constructor()`: `_depositToken` and `_yieldToken` not checked to be different from `address(0x0)`. 1. `Vepoch.setMaxDepositDuration()`: `_newMaxDepositDuration` not checked to be greater than `59` or any other reasonable lower bound.

LowOwnership Can Be RenouncedAcknowledged

If the owner renounces their ownership, all ownable contracts will be left without an owner. Consequently, any function guarded by the `onlyOwner` modifier will no longer be able to be executed.

Show 2 more

LowPrivileged Roles and OwnershipAcknowledged

Specific contracts have state variables, e.g., `owner`, which provide certain addresses with privileged roles. Such roles may pose a risk to end-users. For a detailed list of roles per contract, see the following list: The `Vepoch.sol` contract contains the following privileged roles: 1. `_owner` (`Ownable` inheritance), as initialized during the execution of `constructor` to `msg.sender`: 1. Renounce the role (**and thereby prevent any future calls to the following listed functions!**) by calling `renounceOwnership()`. 1. Transfer the ownership to an arbitrary other address by calling `transferOwnership()`. 1. Forever prevent any future calls to function `setAuthorised()` for changing transfer-authorized addresses by calling `setAuthorisedLocked()`. 1. Forever prevent any future calls to function `setMaxDepositDuration()` for changing the maximum deposit time (up to 315576000 seconds or 3652,5 days) by calling `setMaxDepositDurationLocked()`. 1. Forever prevent any future calls to function `setForfeitRedirection()` for setting/unsetting a forfeit reward redistribution address by calling `setForfeitRedirectionAddressLocked()`. 1. Add/Remove addresses from the `veToken` transfer whitelist (only addresses in that list may transfer `veToken`s via `transfer()`/`transferFrom()`) as long as this function is not locked via `setAuthorisedLocked()` by calling `setAuthorised()`. 1. Change the maximum deposit duration (up to 315576000 seconds or 3652,5 days) as long as this function is not locked via `setMaxDepositDuration()` by calling `setMaxDepositDuration()`. 1. Set/Unset a forfeit reward redistribution address as long as this function is not locked via `setForfeitRedirectionAddressLocked()` by calling `setForfeitRedirection()`. A malicious or compromised owner could in a worst-case therefore: 1. Arbitrarily allow/disallow `veToken` transfers by adding/removing addresses from the authorized whitelist, as long as `setAuthorisedLocked()` has not been called. 1. Denial the service of any future deposits by setting a maximum deposit time (`maxDepositDuration`) of less than 60 seconds, as long as `setMaxDepositDurationLocked()` has not been called.

LowvEPOCH Token Transferal Can Lead to Assets Being Locked ForeverAcknowledged

If an address or third party is authorized to transfer vEPOCH tokens, they may not be able to recover their locked deposits when they transfer vEPOCH tokens. Withdrawing entire deposits requires providing the initially minted amount of vEPOCH tokens. If some of them are transferred but cannot be obtained anymore, (parts of) the deposits cannot be recovered.