Vanilla Contract

CertiK2 Aug 2023Files named in the report

CertiK reviewed Vanilla (Vanilla Contract) on 2 Aug 2023: 22 findings, 0 critical; 17 marked fixed, 1 acknowledged, 4 open.

No open critical findings

Open the report Open Vanilla’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings22

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

Severity	Open	Acknowledged	Fixed
Critical	0	0	0
High	0	1	0
Medium	0	0	0
Low	4	0	17

Low`Distributor.rewardAmount()` uses `msg.sender` inFixed

`rewardAmount()` is supposed to calculate the reward claimable by `account`. However, `msg.sender` is used as argument for `getBoostedAmount()`.

Low`Locker.increaseLockPeriod()` allows increasing tFixed

```sol=144 require(beforeUnlockBlock < afterUnlockBlock, "Vanilla: period"); ``` `increaseLockPeriod()` checks if block number is increasing, however, it's more reasonable to check if stage is.

Low`Reinforcer.numWords` is redundantFixed

`Reinforcer` always uses only one random word in `fulfillRandomWords()`. The `numWords` state field is redundant.

Low`rewardPool` is supposed to be `depositToken`Fixed

The argument `rewardPool` of `Distributor.NewRewardPool` event is in fact `depositToken`. The function `Distributor.rewardPerPeriod()` in fact returns `rewardSince`.

Low`Ticket.pendingTokenIds` is not encapsulatedFixed

`Vanilla721.mint()` directly checks if `!ticket.pendingTokenIds(tokenId)` and then does `ticket.setPendingTokenIds(tokenId)`. However, it is reasonable to perform this check directly in `Ticket.setPendingTokenIds()` to hide the implementation details.

LowDivide Before MultiplyFixed

Performing integer division before multiplication truncates the low bits, losing the precision of calculation. ```solidity=220 uint rewardPerPool = rewardPerPeriod(poolState.lastUpdatedBlock) * poolState.rewardWeight / totalRewardWeight; ``` ```solidity=224 poolState.rewardRate += safe144(rewardPerPool * 1e18 / poolState.totalDeposit); ``` --- ```solidity=53 newMaxRewardWeights[i] = safe112( maxNumber / subtractedPoint * reinforceVanillaFee ); ```

LowInefficient `memory` ParameterFixed

One or more parameters with `memory` data location are never modified in their functions and those functions are never called internally within the contract. Thus, their data location can be changed to `calldata` to avoid the gas consumption copying from calldata to memory. ```solidity=53 function submitTransaction( ``` `submitTransaction` has memory location parameters: `signature`, `data`.

LowLack of sanity checkFixed

`Distributor.constructor()` and `setTokenPerBlock()` don't check if `newStartBlock` and `newTokenPerBlock` are valid. `Locker.setBoostDivider()` doesn't check if `boostDivider` is non-zero.

LowNo Upper Limits for FeesFixed

There is no upper limit restricting parameter of `setFeePercentage()`, potentially enabling up to 100% of fees on transactions.

LowPotential Reentrancy AttackFixed

`Locker.increaseLockPeriod()` is not protected from reentrancy, however, does external calls and updates `accountState.rewardDebt` after that.

Show 7 more

LowPull-Over-Push Pattern In `Vanilla.setAdmin()` FuFixed

`Vanilla.setAdmin()` sets the new admin without guaranteeing they are able to actuate transactions on-chain.

LowUnsafe Integer CastFixed

Type casting refers to changing the variable of one data type into another. The code contains an unsafe cast between integer types, which may result in unexpected truncation or sign flipping of the value. ```solidity=100 uint32(startBlock > block.number ? startBlock : block.number), ``` `startBlock` is declared as `uint`. If it was initialized bigger than `type(uint32).max`, the result of `addRewardPool()` is unexpected. --- ```solidity=260 return blockNumber < startBlock ? uint16(passedStage) : uint16((blockNumber - startBlock) / blockPeriod + passedStage); ``` `blockNumber` is provided by the user as `uint`. The result of type truncation is unexpected. --- ```solidity=23 int i = int(to); ``` `BIT` uses unsafe type casting.

LowUnused codeFixed

`BIT.safe16()`, `Distributor.safe112()`, `Distributor.blocksPerYear` are never used.

LowUnused eventFixed

The event `Reinforcer.NewReinforceLinkFee` is never used.

LowUsage of `transfer()` for Sending Native TokensFixed

It is not recommended to use Solidity's `transfer()` and `send()` functions for transferring native tokens. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=198 payable(msg.sender).transfer(amount); ``` - `Distributor.withdraw` uses `transfer()`.

LowUsage of Magic NumbersFixed

Different magic numbers are used as-is in code.

LowVariables Can Be Declared as `immutable`Fixed

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.