HashEx was commissioned by the DefyFarm team to perform an audit of DefyFarm’s smart contracts. The audit was conducted between May 02 and May 06, 2021.
The audited code is deployed to Binance Smart Chain (BSC): 0x0acbb2C3D3826B82B17C09e2Dfa605b5279E0C63 DefyCoinV2, 0x23185fd9b304f03f7BAd458e6DacECA20Ce5Caa1 ImpermenantLossProtection (DefyILP), 0xB3534770A194226925F90e20e669c8AFbb3B2bff DefyMaster.No documentation was provided.
The purpose of this audit was to achieve the following:
● Identify potential security issues with smart contracts.● Formally check the logic behind given smart contracts.
Information in this report should be used to understand the risk exposure of smart contracts, and as a guide to improving the security posture of smart contracts by remediating the issues that were identified. We found out that DefyCoinV2 token is based on Reflect.finance [1] custom token with an audit report available [2]. DefyMaster is a fork of the MasterChef [3] contract by Sushiswap (audit [4,5]).
Update: DefyFarm team has responded to this report. Individual responses to the high severity issues were added after each item in the section.
Implementation of BEP20 token standard with the custom functionality of auto-yield by burning tokens and distributing the fees on transfers.
Farming contract similar to the MasterChef from Sushiswap.
A contract that intends to compensate for the impermanent loss. Interacts with the ApeSwap.
Implementation of transfer() function in DefyCoinV2 L674 does not allow to input zero amount as it’s demanded in ERC-20 [6] and BEP-20 [7] standards. This issue may break the interaction with smart contracts that rely on full ERC20 support.
Response from Team DEFY: It’s not an issue in our ecosystem because no one needs to transfer 0 amount in any case. Non-zero amount requirement is also implemented in the original RFI contract. (we have closely followed the original RFI contract)
The owner of the DefyCoinV2 contract can redistribute part of the tokens from users to a specific account. For this owner can exclude an account from the reward and include it back later. This will redistribute part of the tokens from holders in profit of the included account. The abuse mechanism can be seen in Appendix C. We suggest lock exclusion/inclusion methods by renouncing ownership.
Response from Team DEFY: We need exclude() function to prevent contracts from getting holders reward. After completely stabilizing the system, we’ll renounce the ownership as you suggest.
The mechanism of removing addresses from auto-yielding in DefyCoinV2 implies a loop over excluded addresses for every transfer operation or balance inquiry. This may lead to extreme gas costs up to the block gas limit and may be avoided only by the owner restricting the number of excluded addresses. In an extreme situation with a large number of excluded addresses transaction gas may exceed maximum block gas size and all transfers will be effectively blocked. Moreover, includeAccount() function relies on the same for() loop which may lead to irreversible contract malfunction.
Response from Team DEFY: The amount of exclusions required to increase gas cost to a point of unusability is far, far more than we would ever have on exclusions. We have only 4–6 contracts to exclude like liquidity pools, farm and ILP contracts.
There are no restrictions on defyPerBlock or BONUS_MULTIPLIER in DefyMaster L1121 and L1130 contracts. The owner of the contract can set the very big reward per block and take the full balance of the contract with the front-run transaction.
Response from Team DEFY: Reward per block depends on the daily volume of the previous day so we can’t limit the reward per block. But we will renounce the ownership of this contract as well when we have stabilized that. (reward sending and reward updating will be done by the reward sender contract so owner can not manually update the reward)
DefyILP L615 contract contains onlyFarm modifier that includes both DefyMaster’s and owner’s addresses. This leads to the overpowered owner, especially in the case of defyTransfer() function. The owner is allowed to transfer any amount of DefyILP balance.
Response from Team DEFY: This includes owner, purposefully, in case the owner needs to do any modifications. Specifically, this will be used for potential upgrades to the impermanent loss contract to add more pools (i.e. defy-bnb), and to add pools back in as necessary upon launch of a new ILP. Without this functionality, the LP pools code get wonkey if, say, PCS does another migration to a new version of LP. Owner is also given access to grabbing the Defy out so he may put into the new ILP contract.
The DefyMaster contract uses the same reward model as the original SushiSwap’s MasterChef ( reward = rewardPerBlock * (delta of block.number) ). At the same time, the contract’s balance is refilled externally and there are no guarantees that its balance will be equal to the generated reward. This may lead to a dangerous situation of inconsistency between the expected value from pendingdefy() and the real transfer amount.
_getDepositValue() function in DefyILP L731 contract returns the result of calculations based on price DEFY/BUSD. The lack of documentation makes it hard to comprehend this behavior, but we believe that the current realization works only for the DEFY pools from the DefyMaster contract.
Update: Team DEFY is informed of the issue and says not defy-busd pools are not supposed to be added for DefyILP.
Response from Team DEFY: By design, this impermanent loss contract is only equipped for defy-busd pools. Possible upgrades may include other pools.
Function getDefyPrice() in DefyILP L744 depends on the DefyCoinV2 address. If the token was deployed to address which value is bigger than the address of BUSD in BSC, the function would return a wrong result. Developers must take care of that in case of future redeployments.
Hardcoded address of Uniswap router in DefyCoinV2 L643. This require statement is useless as the DefyCoinV2 contract is deployed to BSC and the router address is from Ethereum mainnet. We believe it should be the address of ApeSwap router.
PoolInfo structure in DefyMaster L1055 contains token0 and token1. Those are unnecessary as they can be fetched from the lpToken address. Besides, setting them separately can lead to mistakes and inconsistencies in the functioning of the contract in case of wrong sorting order.
Hardcoded addresses of WDev and Wfarm in DefyCoinV2 L502–503. Address hardcoding should be avoided whenever it’s possible.
No need to check if sender is zero address in burn() and transfer() functions of DefyCoinV2 contract.
DefyCoinV2 contract uses pragma ^0.6.0, but can’t be compiled with the 0.6.0 version due to the outdated Address library. Address library is not in use anywhere and should be removed.
setImpermenantLossProtection() function of DefyMaster L1103 should return boolean value, but does not return anything.
setFarmsender() function of DefyMaster L1107 should check it for zero address.
BONUS_MULTIPLIER variable in DefyMaster L1076 is named capitalized, but is not declared constant and could be changed with updateMultiplier() function.
includeAccount() function of DefyCoinV2 L652 performs gas savings by removing the element of excluded[] array. It should also delete the corresponding elements of isExcluded and _tOwned.
Code is very inefficient, gas could be saved almost in every major function, especially on token transfers. Excluding 4 addresses in DefiCoinV2 would cost ~500k of gas for transferStardard() function. DefyILP has unused code such as devAddr and UserInfo. DefyCoinV2 contains getTaxFee() and getMaxTxAmount() which are not in use. Impermanent loss is misspelled through all the contracts.
Reviewed contracts are deployed at0x0acbb2C3D3826B82B17C09e2Dfa605b5279E0C63, 0x23185fd9b304f03f7BAd458e6DacECA20Ce5Caa1, 0xB3534770A194226925F90e20e669c8AFbb3B2bff in BSC. The audited token contract is based on the Reflect.finance model with different fees. Farming contract is based on MasterChef from SushiSwap with additional impermanent loss protection via DefyILP contract.
6 high severity issues were found. DefyFarm team is informed of them and has responded to the issues. You can find the response in updates after corresponding issues.
Issues number 2, 3, 4, 5, 7 can be mitigated if the owner of the contracts is trusted and uses the contracts with extra care.
Audit includes recommendations on the code improving and preventing potential attacks.
HashEx website: https://hashex.orgLinkedIn https://www.linkedin.com/in/dmitrymishunin/