Decubate

CertiK30 May 2023Files named in the report

Audit

CertiK reviewed Decubate on 30 May 2023: 26 findings, 0 critical; 16 marked fixed, 10 open.

No open critical findings

Open the report Open Decubate’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings26

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

Severity	Open	Fixed
Critical	0	0
High	2	0
Medium	0	4
Low	8	12

MediumCrowdfunding Contract only Supports Payment TokenFixed

The crowdfunding contract allows users to buy designated tokens with stablecoins. According to the test cases and comments, it is assumed that the main stablecoin used is BUSD. However, the contract may fail to perform the given functionalities if the payment token's decimal setting is not 18, such as USDC. For example, in the function `fundAgreement()`, the price of the on-sale token is determined by the following implementation ```solidity=307 uint256 price = (dcbAgreement.hardcap * 10 ** paymentDecimals) / dcbAgreement.totalTokenOnSale; ``` The concern is the price could be inaccurate and can even be zero due to the truncation values when performing division.

MediumIncompatibility with Deflationary TokensFixed

When transferring deflationary ERC20 tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee), only 90 tokens actually arrived to the contract. However, a failure to discount such fees may allow the same user to withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction. Reference: https://thoreum-finance.medium.com/what-exploit-happened-today-for-gocerberus-and-garuda-also-for-lokum-ybear-piggy-caramelswap-3943ee23a39f ```solidity=146 token.safeTransferFrom(msg.sender, address(this), _amount); ``` - Transferring tokens by `_amount`. ```solidity=160 poolInfo.shares = poolInfo.shares + _amount; ``` - The `_amount` appears to be used for bookkeeping purposes without compensating the potential transfer fees. --- ```solidity=146 token.safeTransferFrom(msg.sender, address(this), _amount); ``` - Transferring tokens by `_amount`. ```solidity=160 poolInfo.shares = poolInfo.shares + _amount; ``` - The `_amount` appears to be used for bookkeeping purposes without compensating the potential transfer fees.

MediumPotential Reward LossFixed

The `stake()` function is used to stake `_amount` tokens to the pool. ```solidity function stake(uint _amount) external override returns (bool) { uint stopDepo = poolInfo.endDate - poolInfo.lockPeriodInDays * 1 days; require(block.timestamp <= stopDepo && block.timestamp >= poolInfo.startDate, "Not in stake period"); require(poolInfo.totalDeposit + _amount <= poolInfo.hardCap, "Pool is full"); IERC20 token = IERC20(poolInfo.token); require(token.allowance(msg.sender, address(this)) >= _amount, "Decubate : Set allowance first!"); token.safeTransferFrom(msg.sender, address(this), _amount); User storage user = users[msg.sender]; if (user.depositTime == 0) { participants.push(msg.sender); } user.depositTime = block.timestamp; user.shares = user.shares + _amount; _reinvest(); poolInfo.totalDeposit = poolInfo.totalDeposit + _amount; poolInfo.shares = poolInfo.shares + _amount; emit Stake(msg.sender, _amount, block.timestamp); return true; } ``` However, the function does not check the input `_amount`, which may lead to a potential reward loss issue. The auditing team would like to confirm with the team that it is designed as expected.

MediumPotentially Incorrect Unstake Withdrawal AmountsFixed

Users can earn rewards by staking and locking tokens into the `Farm` contract, and after a certain time period has passed, users can withdraw their tokens along with the reward by calling the `unStake()` function. ```solidity=198 function unStake() external override returns (bool) { _reinvest(); // get rewards not yet paid out User storage user = users[msg.sender]; // if lock period not over, withdraw with no rewards uint withdrawAmount = canClaim(msg.sender) ? (poolInfo.totalDeposit * user.shares) / poolInfo.shares : user.shares; uint earnedReward = withdrawAmount - user.shares; // if fee taker is set up trasfer amount to fee wallet if (feePercent > 0 && earnedReward > 0) { uint fee = (earnedReward * feePercent) / 1000; safeTOKENTransfer(poolInfo.token, feeAddress, fee); earnedReward = earnedReward - fee; // update earned reward } poolInfo.totalDeposit = poolInfo.totalDeposit - withdrawAmount; poolInfo.shares = poolInfo.shares - user.shares; user.shares = 0; user.totalWithdrawn = user.totalWithdrawn + withdrawAmount; user.totalRewards = user.totalRewards + earnedReward; safeTOKENTransfer(poolInfo.token, msg.sender, withdrawAmount); emit Unstake(msg.sender, withdrawAmount, block.timestamp); return true; } ``` The issue is when calling `unStake()`, users can withdraw the amount `withdrawAmount` as shown in line 222 of the code. However, since an **additional** fee may be charged (especially in the case when rewards are accrued), the contract will send an extra amount of `fee` to the `feeAddress`, which creates an issue in how the `poolInfo.totalDeposit` is updated (does not consider the fees). Therefore, there is a chance that the contract may run out of tokens due to extra fees that were unaccounted for.

LowAccess of storage instead copying to memoryFixed

In many cases there are structures that are created and stored in memory to copy and store data from storage. Since the EVM needs to continually load and copy data from storage and then store it to memory, the constant use of expensive operations such as SSLOAD, MLOAD, and MSTORE lead to the continual increase in the gas usage. Instead of copying large amounts of data and storing it into memory, it would be more efficient to simply retrieve the data directly from storage.

LowArrangement of data types within structFixed

There are various locations in the code where storage is inefficient and can potentially use up more storage than needed.

LowCheck-effect-interaction Pattern ViolationFixed

In the `Farm` contract, the `unStake()` function has a violation of the check-effects-interaction pattern. The vulnerability is mainly related to the calling of the `safeTOKENTransfer()` function within the `unStake()` function. ```solidity=212 safeTOKENTransfer(poolInfo.token, feeAddress, fee); ``` In order to ensure that there are no issues, external calls such as this should be batched together after status variables such as amounts and balances are updated. It is important to note that the address where fees are sent is the `feeAddress`, this address is set by the farm creator in the `Factory` contract through the `Farm.initialize()` function.

LowConsider use of mappings instead of loopsFixed

In the `MockDCBTiers` contract implementation of `getTierOfUser` it was noted that expensive looping is used to extract the information to return to users. ```solidity function getTierOfUser(address addr) public pure returns (bool flag, uint256 pos, uint256 multiplier) { for (uint256 i = 1; i <= 300; i++) { if (addr == vm.addr(i)) { return (true, (i + 1) % 10 + 2, 1); } } } ```

LowIncomplete Input ValidationFixed

Files: - DCBTokenClaim.sol Commit hash: 7a4b24893cf2a661f5a66e2ed797a88f05d4d73b The function `registerByManager()` only checks `_users.length` against `_tierOfUser.length`, without checking if `_tierOfUser.length` equals `_multiOfUser.length`, which may lead to a potential index-out-of-range error. ```solidity=145 require(_users.length == _tierOfUser.length, "Invalid input"); ```

LowMissing Emit EventsFixed

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

Show 6 more

LowMissing Zero Address ValidationFixed

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ### Farm.sol - `_token` is not zero-checked before being used. ```solidity=109 poolInfo.token = _token; ``` - `_feeAddress` is not zero-checked before being used. ```solidity=113 feeAddress = _feeAddress; ``` --- ### DCBFactory.sol - `_c.saleTokenAddr` is not zero-checked before being used. ```solidity=113 e.tokenAddress = _c.saleTokenAddr; ``` - `_token` is not zero-checked before being used. ```solidity=142 e.tokenAddress = _token; ``` `_newVesting`, `_newCrowdfunding` and `_newTokenClaim` are not zero-checked before being used. ```solidity=172 function changeImplementations( address _newVesting, address _newTokenClaim, address _newCrowdfunding ) external onlyRole(DEFAULT_ADMIN_ROLE) { vestingImpl = _newVesting; crowdfundingImpl = _newCrowdfunding; tokenClaimImpl = _newTokenClaim; } ``` ### DCBTokenClaim.sol ```solidity=78 function initialize(Params memory p) external initializer { _walletStore = IDCBWalletStore(p.walletStoreAddr); _investment = IDCBInvestments(p.investmentAddr); _tiers = IDCBTiers(p.tiersAddr); _rewardToken = IERC20(p.rewardTokenAddr); _vesting = IDCBPlatformVesting(p.vestingAddr); //... } ```

LowRecommended use of Calldata input ParametersFixed

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.

LowRedundant Check for `block.timestamp`Fixed

Files: - DCBTokenClaim.sol Commit hash: 7a4b24893cf2a661f5a66e2ed797a88f05d4d73b The `require` statement checks whether the `block.timestamp` is between `claimInfo.startDate` and `claimInfo.endDate` or not. Since `block.timestamp` won't update during a function call, performing the check for every user is unnecessary. ```solidity=148 for (uint256 i = 0; i < len; i++) { require(_tierOfUser[i] >= claimInfo.minTier, "Minimum tier required"); require(_walletStore.isVerified(_users[i]), "User is not verified"); require(userAllocation[_users[i]].active == 0, "Already registered"); require( block.timestamp <= claimInfo.endDate && block.timestamp >= claimInfo.startDate, "Registration closed" ); ```

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

The return value of the transfer()/transferFrom() call is not checked. ### DCBPlatformVesting.sol ```solidity=175 paymentToken.transfer(msg.sender, refundAmount); ``` --- ```solidity=201 paymentToken.transfer(innovator, amountPayment); ``` --- ```solidity=204 vestedToken.transfer(innovator, amountTokenToReturn); ``` --- ```solidity=209 paymentToken.transfer(paymentReceiver, decubateFee); ``` --- ```solidity=251 vestedToken.transfer(_wallet, releaseAmount); ``` --- ```solidity=268 paymentToken.transferFrom(_wallet, address(this), paymentAmount); ```

LowUnlocked Compiler VersionFixed

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.

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.