Chronos Audit 2

CertiK21 Apr 2023Files named in the report

CertiK reviewed Chronos (Chronos Audit 2) on 21 Apr 2023: 32 findings, 0 critical; 21 marked fixed, 1 acknowledged, 10 open.

No open critical findings

Open the report Open Chronos’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings32

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

Severity	Open	Acknowledged	Fixed
Critical	0	0	0
High	0	0	2
Medium	0	1	6
Low	10	0	13

HighState Variables in Upgradeable Contracts are InitFixed

State variables initialized when declared are equivalent to initializing them inside the constructor. Therefore, initializing state variables when declared in an upgradeable contract has no actual effect since the constructor of an upgradeable contract is never called. ```solidity=82 uint PRECISSION = 1000; ```

HighTransfer Tokens from Airdrop Contract Without AppFixed

The `_deposit_for` function transfers tokens from the `AirdropClaimChrNFT` contract without approval. Therefore, if the airdrop boost feature is triggered when locking tokens into the `VotingEscrow`, calling the `IERC20(token).transferFrom` method will result in an error.

Medium`supply` is not updatedFixed

In the `_deposit_for()` function, when a user meets the bonus airdrop conditions, the user's locked amount increases, but the `supply` does not. However, in the `withdraw()` function, the `supply` decreases, which can cause subsequent withdrawals to fail because the locked amount may be greater than the `supply`.

MediumFunction `transfer` of Contract `Chronos` May CauFixed

Any invocation of `transfer(recipient, amount)` must fail if it causes the balance of the `recipient` address to overflow. This is not ensured by `Chronos.transfer`, i.e. it admits transfers that lead to an overflow of the recipient's balance and thereby to an irrecoverable loss of large amounts of tokens.

MediumFunction `transferFrom` of Contract `Chronos` MayFixed

Any call of the form `transferFrom(from, dest, amount)` with a value in `amount` whose transfer would lead to an overflow of the balance of the `dest` address should fail. However, in Chronos, some invocations of `transferFrom(from, dest, amount)` cause an overflow in the recipient's balance.

MediumNFT Buyer May Not Be Able to Claim WETHFixed

The claimable rewards for ChrNFT minters are calculated using the following formula: ```solidity=117 _reward += _fee * weight / (_tot - _resAmnt); ``` The total rewards are divided by the sold ChrNFT number, which means the rewards are shared by NFT buyers but not by reserved NFT minters. Meanwhile, the `claim` function does not check whether the reward claimer is an NFT buyer. If the reserved NFT owner claims rewards before buyers, some NFT buyers will not be able to claim rewards.

MediumNFT Buyers Claim Less Airdrop Tokens Than ExpecteFixed

```solidity=118 uint timeElapsedSinceStart = block.timestamp - START_VESTING; uint claimable Percent = (timeElapsedSinceStart*PRECISION)/VESTING_PERIOD; userInfo.CHRClaimable = _totalCHR*claimablePercent/PRECISION - claimedCHR[_who]; ``` If the value of `timeElapsedSinceStart * PRECISION` is greater than `VESTING_PERIOD`, the calculated `claimablePercent` exceeds 100%. Consequently, if some NFT buyers do not claim all the airdrops, other NFT buyers can claim more airdrops than `_ogMints * _amountPerNFT`. After the vesting period has ended for some time, NFT buyers who have not claimed any airdrops can only claim a portion of the airdrops that originally belonged to them.

Mediumunreset `supply`Fixed

The `merge()` function merges the value of `_from` NFT into the `_to` NFT and burns the `_from` NFT. However, the `supply` variable in the `_deposit_for()` function, which represents the total value of all NFTs deposited into the contract, is added again during the merge process. As a result, the `supply` value will be larger than before, since the value of `_from` is not deducted from the `supply` after the merge is complete.

LowCheck Effect Interaction Pattern ViolatedFixed

The order of external call and storage manipulation must follow the check-effect-interaction pattern. The tokens are transferred before the state is updated.

LowLack of Storage Gap in Upgradeable ContractFixed

There is no storage gap preserved in the logic contracts. Any logic contract that acts as a base contract that needs to be inherited by other upgradeable children should have a reasonable size of storage gap preserved for the new state variable introduced by the future upgrades.

Show 10 more

Lowmismatch between the function definition and theFixed

The `EIP712Domain()` function specifies parameters `name`, `chainId`, and `verifyingContract`. However, the parameter `version` is passed in when calling the "abi.encode()" function, causing a mismatch between the function definition and the passed parameter. ```solidity=1484 bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); ``` ```solidity=1484 bytes32 domainSeparator = keccak256( abi.encode( DOMAIN_TYPEHASH, keccak256(bytes(name)), keccak256(bytes(version)), block.chainid, address(this) ) ); ```

LowMissing check `attachments[_tokenId] > 0`Fixed

The function lacks a sanity check to ensure that `attachments[_tokenId]` is greater than 0. This omission could cause the function to revert when `attachments[_tokenId]` is 0.

LowNo Upper Limit in `setReferralFee` functionFixed

There are no upper boundaries for `setReferralFee` which is used to set the referral fee. It is possible to set the total fee rate up to any arbitrary amount.

LowRedundant StatementsFixed

The linked statements do not affect the functionality of the codebase and appear to be either remnants of test code or older functionality.

LowRedundant Statements When Calculating Airdrop BooFixed

The airdrop `_boostValue` to be added to the locked token amount is calculated by the formula `_boostValue = _value*minBonusPercent / PRECISSION`, where `_value` is the amount to be deposited and `minBonusPercent` is set by the `team` role. The calculations from lines 772 to 780 are irrelevant to `_boostValue` and are unnecessary. Furthermore, when `_bonusTime` is smaller than `maxBonusTime`, setting `_bonusPercent` to `maxBonusPercent` in line 775 is incorrect. In addition, `_bonusPercent` is written twice, but not used in between. ```solidity=776 _bonusPercent = maxBonusPercent; ``` ```solidity=780 _bonusPercent = _bonusTime*PRECISSION/maxBonusTime; ```

LowRemove `token` from `tokenToRoutes` TwiceFixed

On line 194, `token` has been removed from the `tokenToRoutes` map and is removed again on line 196.

LowState Variable Should Be Declared ConstantFixed

State variables that never change should be declared as `constant` to save gas. --- ```solidity=18 address public redemptionReceiver; ``` - `redemptionReceiver` should be declared `constant`. --- ```solidity=19 address public merkleClaim; ``` - `merkleClaim` should be declared `constant`. --- ```solidity=26 uint public DISTRIBUTION = 7 * 86400; ``` - `DISTRIBUTION` should be declared `constant`. --- ```solidity=82 uint PRECISSION = 1000; ``` - `PRECISSION` should be declared `constant`. --- ```solidity=16 bool public init; ``` - `init` should be declared `constant`. --- ```solidity=21 uint256 public tokenPerSec; ``` - `tokenPerSec` should be declared `constant`.

LowThe airdrop reward will be changed based on the `Fixed

The amount that users can claim from the airdrop depends on the value of the 'totalAirdrop'. When the 'owner' or 'depositors' add or remove tokens from the contract, the claimable amount will change. This means that users might be able to claim more than once if the 'totalAirdrop' increases after the user claims for the first time. For most projects that perform an airdrop, it usually will be a one-time thing, and the amount will be pre-determined. ```solidity=99 uint _totalSupply = IChrNFT(chrNFT).totalSupply(); uint _amountPerNFT = totalAirdrop/_totalSupply; uint _amount = _ogMints * _amountPerNFT; uint _veShare = (VE_SHARE * _amount) / PRECISION; userInfo.veCHRTotal = _veShare; userInfo.veCHRClaimed = claimedVeCHR[_who]; userInfo.veCHRLeft = _veShare - userInfo.veCHRClaimed; userInfo.veCHRClaimable = userInfo.veCHRLeft; ```

LowUnnecessary `require` StatementFixed

Comparisons that are always true or always false may be incorrect or unnecessary. The variable "_pid" is unsigned integer, which will always be larger than 0. ```solidity=109 require(_pid >= 0, "zero"); ```

LowUnused State VariableFixed

One or more state variables are never used in the codebase. Variable `DURATION` in `VoterV2_1` is never used in `VoterV2_1`. Variable `redemptionReceiver` in `Chronos` is never used in `Chronos`.

Show 1 more

LowUpdate the parameter `totalAirdrop` without checkFixed

The `deposit()` function increases the `totalAirdrop` by the corresponding amount when the caller transfers tokens to the contract and decreases it when the caller withdraws tokens. However, there is an issue where the `totalAirdrop` is also decreased if the input parameter `_token` is not equal to `token`. If someone transfers another token to the contract and the caller withdraws it, it would lead to an incorrect amount calculation in the `claimable` function, and the `token` tokens cannot be withdrawn.