Flokifi

CertiK14 Mar 2023Files named in the report

Audit

CertiK reviewed FlokiFi on 14 Mar 2023: 25 findings, 0 critical; 17 marked fixed, 8 open.

No open critical findings

Open the report Open FlokiFi’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings25

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

Severity	Open	Fixed
Critical	0	0
High	3	4
Medium	0	2
Low	5	11

High`_lastAdded` not updatedFixed

The private variable `_lastAdded` has not been updated anywhere, hence `_lastAdded[lpToken]` will always be zero, as a result, the `_sellDelay` check will be easily passed.

HighLack of Fallback or Receive Function When ReceiviFixed

The common UniswapV2 functionality allows contracts to swap tokens for a native token such as ETH. In order to receive the native token, the contract must implement either a `receive()` or `payable fallback` function. If the contract's Solidity version is below 0.6.x, the contract must implement an unnamed payable function.

HighSome `_lpTokens` may never in the `tokenBatch`Fixed

According to the following code: ```solidity function checkUpkeep( bytes memory /* checkData */ ) external view override returns (bool upkeepNeeded, bytes memory performData) { ... uint256 loopLimit = _lpTokens.length(); if (loopLimit > _maxSellBatchSize) { loopLimit = _maxSellBatchSize; } address[] memory tokenBatch = new address[](loopLimit); uint256 tokenBatchCounter = 0; for (uint256 i = 0; i < loopLimit; i++) { address lpToken = _lpTokens.at(i); if ((_lastAdded[lpToken] + _sellDelay) < block.timestamp) { tokenBatch[tokenBatchCounter] = lpToken; } } ... } ``` , if the length of `_lpTokens` is 30, while the `_maxSellBatchSize` is 20, then the `loopLimit` will be 20. In this case, the number from 20 to 29 of `_lpTokens` will never be added to the `tokenBatch` no matter how many times the `checkUpkeep()` is called.

HighThe `performData` should not be manually passedFixed

The function `performUpkeep()` should perform the _removeLiquidity() for the LP tokens in the LPTokenProcessorV1 contract. However, this function takes the parameter `performData` as the target. The truth is the LP token in the parameter `performData` may not exist in the LPTokenProcessorV1 contract.

MediumThe `tokenBatchCounter` should be addedFixed

According to the following code: ```solidity function checkUpkeep( ... address[] memory tokenBatch = new address[](loopLimit); uint256 tokenBatchCounter = 0; for (uint256 i = 0; i < loopLimit; i++) { address lpToken = _lpTokens.at(i); if ((_lastAdded[lpToken] + _sellDelay) < block.timestamp) { tokenBatch[tokenBatchCounter] = lpToken; } } ``` , the `tokenBatchCounter` should be added after `tokenBatch[tokenBatchCounter] = lpToken`.

MediumWrong calculation in `getAvailableTokens()`Fixed

According to the following code, ```solidity function getAvailableTokens() public view returns (address[] memory, uint256[] memory) { ... // Send over all remaining tokens in the case where the unlock // timestamp has already expired. Otherwise vest linearly. if (block.timestamp >= unlockTimestamp) { amounts[i] = IERC20(tokens[i]).balanceOf(address(this)); } else { amounts[i] = (tokenAmounts[i] * (block.timestamp - lastVestingTimestamp)) / (unlockTimestamp - lockTimestamp); ... } ``` , when `block.timestamp < unlockTimestamp`, the `amounts[i]` will be calculated linearly in terms of the `block.timestamp - lastVestingTimestamp`, however, according to the contract `PaymentModuleV1`, when the token is a LP token, the amount of tokens deposited in the contract should be `fungibleTokenDeposits[i].amount - lpAmounts[j]` instead of the `tokenAmounts[i]`.

LowAssembly UsageFixed

```solidity=161 assembly { mstore(paymentTokens, sub(mload(paymentTokens), sub(totalTokens, paymentCounter))) mstore(paymentAmounts, sub(mload(paymentAmounts), sub(totalTokens, paymentCounter))) } ```

LowMissing Emit EventsFixed

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

LowMissing Error MessagesFixed

The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

LowMissing Zero Address ValidationFixed

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=15 vaultFactory = vaultFactoryAddress; ``` - `vaultFactoryAddress` is not zero-checked before being used. --- ```solidity=43 USDT = usdtAddress; ``` - `usdtAddress` is not zero-checked before being used. --- ```solidity=38 FLOKI = flokiAddress; ``` - `flokiAddress` is not zero-checked before being used. --- ```solidity=42 treasury = treasuryAddress; ``` - `treasuryAddress` is not zero-checked before being used. --- ```solidity=169 treasury = newTreasury; ``` - `newTreasury` is not zero-checked before being used.

Show 7 more

LowPrivileges on the `KeyHolder` roleFixed

The role 'KeyHolder' has lots of privileges to control the corresponding vault, we would like the client to provide with us more information about the 'KeyHolder' role.

LowRedundant `oldPrice` and `oldBasisPoints`Fixed

The local variable `oldPrice` and `oldBasisPoints` are defined but not used anywhere in the function, hence they are redundant.

LowSending Ether to Arbitrary DestinationsFixed

The function call that sends Ether to an arbitrary address is unprotected. ```solidity=264 try router.swapExactETHForTokens{ value: ETHGained }(0, path, receiver, block.timestamp) returns ( uint256[] memory /* amounts */ ) {} catch ( bytes memory /* lowLevelData */ ) { return false; } ```

LowUnchecked ERC-20 `transfer()`/`transferFrom()` CaFixed

The return value of the transfer()/transferFrom() call is not checked. ```solidity=27 token.transferFrom(address(this), msg.sender, token.balanceOf(address(this))); ``` --- ```solidity=34 IERC20(_tokenAddress).transferFrom(address(this), msg.sender, _tokenAmount); ``` --- ```solidity=40 token.transferFrom(address(this), msg.sender, token.balanceOf(address(this))); ``` --- ```solidity=69 IERC20(_tokenAddress).transferFrom(address(this), msg.sender, _tokenAmount); ```

LowUnchecked Low-level CallFixed

The low-level `call` function returns the status of the call as the first variable in the returned tuple. The status of the `call` is not asserted to be `true`, which would treat the low-level call as a success even when it reverted. ```solidity=201 payable(msg.sender).call{value: amount}(""); ```

LowUnused EventFixed

```solidity=31 event LPTokenCleared(address indexed tokenAddress); ``` - `LPTokenCleared` is declared in `LPTokenProcessorV1` but never emitted.

LowVariables That Could Be Declared as ImmutableFixed

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.