BendDAO Audit Report

Our auditing service includes the following two stages:

• Pre-Audit Consulting Service

• Smart Contract Auditing Service

1. Pre-Audit Consulting Service
   • [Price Oracle Design Discussion Meeting]

     The Verilog Solutions team discussed with the BendDAO team regarding its collateral price oracle design. Throughout the discussion, the Verilog Solutions team conducted a review of the current oracle design and provided feedback and suggestions to the BendDAO team. It is acknowledged by both teams that the price oracle should be as simple as possible to ensure the reliability of data and reduce the attack surface.

   • [Protocol Security & Design Discussion Meeting]

     The Verilog Solutions team discussed with the BendDAO team about off-chain component executions and the security best practice for securing the off-chain components. Verilog Solutions team also provided feedback and suggestions on how to ensure reliability for mission-critical off-chain components including the price oracle service and monitoring.

2. Smart Contract Auditing Service

   The Verilog Solutions team analyzed the entire project using a detailed-oriented approach to capture the fundamental logic and suggested improvements to the existing code. Details can be found under Findings & Improvement Suggestions.

• Code Assessment
  • We evaluate the overall quality of the code and comments as well as the architecture of the repository.
  • We help the project dev team improve the overall quality of the repository by providing suggestions on refactorization to follow the best practice of Web3 software engineering.

• Code Logic Analysis
  • We dive into the data structures and algorithms in the repository and provide suggestions to improve the data structures and algorithms for the lower time and space complexities.
  • We analyze the hierarchy among multiple modules and the relations among the source code files in the repository and provide suggestions to improve the code architecture with better readability, reusability, and extensibility.

• Business Logic Analysis
  • We study the technical whitepaper and other documents of the project and compare its specification with the functionality implemented in the code for any potential mismatch between them.
  • We analyze the risks and potential vulnerabilities in the business logic and make suggestions to improve the robustness of the project.

• Access Control Analysis
  • We perform a comprehensive assessment of the special roles of the project, including their authorities and privileges.
  • We provide suggestions regarding the best practice of privilege role management according to the standard operating procedures (SOP).

• Off-Chain Components Analysis
  • We analyze the off-chain modules that are interacting with the on-chain functionalities and provide suggestions according to the SOP.
  • We conduct a comprehensive investigation for potential risks and hacks that may happen on the off-chain components and provide suggestions for patches.

Our auditing for BendDAO covered the repository:

• https://github.com/BendDAO/bend-lending-protocol : commit hash 087c284fda18e9d7fd3a07de384a2245d5eab6ea

  including both smart contracts source code and the markets’ configuration for Bend Protocol.

contains the smart contracts source code and markets’ configuration for Bend Protocol.

LendPool.sol: It is the main entry point into the BendDAO Protocol. Most interactions with the BendDAO Protocol happen via the LendPool.sol. Users can either deposit supported assets to earn interest or deposit supported NFTs to borrow reserve assets.

LendPoolLiquidator.sol: It contains the main logic of the auction and liquidation functions. LendPool.sol delegates calls to LendPoolLiquidator.sol for auction(), redeem() and liquidate().

LendPoolLoan.sol: It is the NFT loan manager of the protocol, which generates a unique loan when NFTs are used as collateral in borrowing, and maintains the relationship between the NFT and the loan.

BToken.sol: The BToken is an interest-bearing transferrable token minted and burned upon a deposit and withdrawal. The value of BToken is pegged to the value of the corresponding deposited asset at a 1:1 ratio.

DebtToken.sol: The DebtToken is an interest-accruing non-transferrable token minted and burned on borrow and repay. It represents debts owed by a token holder.

contains the smart contract source code to provide lending protocol with a market valuation of the underlying asset.

The BendDAO team designed a filter algorithm to fetch the floor price data from OpenSea and LooksRare.

1. The NFT Oracle is currently maintained and operated by the BendDAO team. The data fetching and filtering algorithm scripts are running off-chain.

2. In the future, the Bend governance mechanisms will manage the selection of data sources.

BoundNFTs are promissory-note tokens that are minted and burned upon borrowing and repaying, representing the NFT used as collateral owed by the token holder, with the same token ID and metadata. It has a flash claim feature for users to claim airdrops for NFT holders.

There are two admin roles in BNFT contracts:

• _owner: handles ownership and admin;

• _claimAdmin: claims and executes ERC20/ERC721/ERC1155 airdrops.

BendDAO Protocol also has periphery contracts such as WETHGateway.sol and PunkGateway.sol:

• WETHGateway.sol: LendPool only supports ERC20 tokens as underlying reserve assets. WETHGateway.sol provides an interface that allows users to deposit native tokens such as ETH directly without the need to swap between the native token and the wrapped native token themselves.

• PunkGateway.sol: Lending pool only supports interactions with the standard ERC721 token. NFT CryptoPunks is not ERC721 compatible. PunkGateway helps users to wrap and unwrap CryptoPunks and interact with lend pool.

none ; )

1. LendPool.sol : Time elapsed in the PAUSE state needs to be deducted in Redeem and Auction functions.

   Severity	Medium
   Source	contracts/protocol/LendPool.sol: #L408 contracts/protocol/LendPool.sol: #L431
   Status	Resolved in commit 64b7861e48cf0156d5f203e3a39166f22ae36dd0.

   • Description

     Time-sensitive functions such as redeem and auction do not deduct time elapsed in the paused state.

     If an auction starts and then the protocol enters into a pause state, the time elapsed during the pause period is still counted in time-sensitive functions such as auction and redeem.

     As a result, any time elapsed in the pause state will erode into the 48-hour grace period of the auction state. Bidders will have a shorter time window than the 48-hour grace period to bid, and the borrower will have a shorter time window than the expected to redeem.

   • Exploit Scenario

     There are 2 conditions that need to be fulfilled:

     1. There is an undergoing auction;

     2. The project team triggered the pause function and put the protocol into a pause state.

     As a result, any time elapsed in the pause state will erode into the 48-hour grace period of the auction state. This may result in:

     1. Bidders will have a shorter window to bid,

     2. Borrowers will have a shorter window to redeem.
   • Recommendations

     After unpause, the protocol needs to calculate the duration of the paused period and update auction timestamp to reflect the time elapsed in the paused state.

   • Results

     Pause/unpause is rarely executed unless there is an emergency situation. Resolved in commit 64b7861e48cf0156d5f203e3a39166f22ae36dd0.

1. Unnecessary Require and Redundant Parameter

   Severity	Low
   Source	contracts/protocol/LendPoolLiquidator.sol
   Status	Acknowledged.

   • Description

     In LendPoolLiquidator.sol, there is an unnecessary require in redeem() function and redundant parameter bidFine.

     require(vars.bidFine <= bidFine, Errors.LPL_BID_INVALID_BID_FINE); 

     Variable bidFine is an input provided by the user, while vars.bidFine is a variable calculated from the contract’s internal states. vars.bidFine is actually referenced in other places, while bidFine is unused anywhere except this requirement check. Therefore, this requirement check is unnecessary, as the input variable being checked bidFine is not used anywhere in other places. One of the consequences of having this unnecessary requirement check is that if the user inputs a value of the variable bidFine that is smaller than vars.bidFine, the function call will revert. Meanwhile, the incorrect bidFine would not have an effect anywhere, as bidFine is not used anywhere else except this require a check.

     require(vars.repayAmount <= vars.maxRepayAmount, Errors.LP_AMOUNT_GREATER_THAN_MAX_REPAY);

     The current design will cause a revert in the cases where the user input amount is bigger than the maxRepayAmount. Thus, the frontend must calculate the correct amount to be used in the function call. If the users are constructing the transaction themselves (i.e., in the case that the frontend is down), the users must calculate the correct amount. Otherwise, the transaction will fail. As an alternative design that can improve user experience, the function can just assign the repayAmount with the maxRepayAmount if the amount that the user inputs are bigger than the maxRepayAmount.

   • Exploit Scenario

     N/A

   • Recommendation

     Remove these two require and assign the repayAmountwith the maxRepayAmountif the amount the user inputs is bigger than the maxRepayAmount.

   • Results

     Acknowledged. The BendDAO team decided to keep this design.

2. Unused State in LoanState Enumerator

   Severity	Low
   Source	contracts/libraries/types/DataTypes.sol
   Status	Acknowledged.

   • Description

     LoanState enumerator in DataTypes.sol has a state variable Created which has never been used across the entire repository.

     DataTypes.sol stored all the pre-defined configuration-related data struct and enumerator. LoanState enumerator defines the current state of a loan. There are in total 6 pre-defined states:

     1. None - Default value, this means zero value in loan;

     2. Created - Loan data has been stored, but not initiated yet;

     3. Active - The loan has been initialized, funds have been delivered to the borrower and the collateral is held;

     4. Auction - The loan is in the auction, and the highest price liquidator will get a chance to claim it;

     5. Repaid - The loan has been repaid and the collateral has been returned to the borrower, which is a terminal state;

     6. Defaulted - The loan was delinquent and collateral was claimed by the liquidator, which is a terminal state.

     However, LoanState.Created has never been used. LoanState is changed to Active after createLoan(). Having unused states might become confusing for subsequent development work.

   • Exploit Scenario

     N/A

   • Recommendation

     Remove the unused state.

   • Results

     Acknowledged. It’s from the previous code design. The BendDAO team decided to keep this design.

3. Redundant Parameter

   Severity	Low
   Source	contracts/protocol/LendPoolLiquidator.sol: L286
   Status	Acknowledged.

   • Description

     In LendPoolLiquidator.sol, the function liquidate() has an input parameter called amount, which is redundant and can be easily bypassed.

     Function liquidate() in LendPoolLiquidator.sol can be called to liquidate a non-healthy position. The function caller (liquidator) buys the collateral asset from the user that is liquidated and receives the collateral asset. There are 3 parameters that the caller needs to pass in, but only the first two are necessary:

     1. nftAsset - The address of the underlying NFT used as collateral;

     2. nftTokenId - The token ID of the underlying NFT used as collateral;

     3. amount - The amount of ETH to be paid by the liquidator.

     However, the function input parameter amount is only referenced in the require statement. The actual amount that the liquidator needs to pay is calculated internally in the liquidate() function.

     Therefore, this requirement check is unnecessary, as the input variable being checked amount is not used anywhere in other places. One of the consequences of having this unnecessary requirement check is that if the user inputs an amount that is greater than vars.extraDebtAmount, the function call will revert. Meanwhile, the incorrect amount would not have an effect anywhere, as amount is not used anywhere else except this require statement.

     The frontend is responsible for calculating the correct amount used in the function call. If the users are constructing the transaction themselves (i.e., in the case that the frontend is down), the users must calculate the correct amount. Otherwise, the transaction will fail.

   • Exploit Scenario

     N/A

   • Recommendation

     Parameter amount in function liquidate() can be removed.

   • Results

     Acknowledged. This parameter is mainly for the WETHGateWay.sol contract, checking the msg.value passing from the gateway contract to the main lend pool contract. The BendDAO team decided to keep this design.

4. Function executeOperation()uses a method from non-standard EIP721

   Severity	Low
   Source	boundnft-protocol/contracts.misc/AirdropFlashLoanReceiver.sol: L106
   Status	Resolved.

   • Description

     Function executeOperation() uses IERC721Enumerable(token).tokenOfOwnerByIndex(), which is not supported by standard EIP721.

     AirdropFlashLoanReceiver.sol is designed for letting NFT depositors receive their deposited NFT’s airdrops. It can handle multiple situations, such as claim airdrop ERC20 token, ERC721 NFT, or ERC1155.

     Function executeOperation()main logic is the following:

     1. Allow the operator to transfer borrowed NFTs back to BNFT contracts;

     2. Call project airdrop contract;

     3. Transfer airdrop tokens to the borrower.

     However, in STEP 3:

     else if (vars.airdropTokenTypes[typeIndex] == 2) {
       // ERC721
       vars.airdropBalance = IERC721(vars.airdropTokenAddresses[typeIndex]).balanceOf(address(this));
       for (uint256 i = 0; i < vars.airdropBalance; i++) {
         vars.airdropTokenId = IERC721Enumerable(vars.airdropTokenAddresses[typeIndex]).tokenOfOwnerByIndex(
           address(this),
           0
         );
         IERC721Enumerable(vars.airdropTokenAddresses[typeIndex]).safeTransferFrom(
           address(this),
           initiator,
           vars.airdropTokenId
         );
       }
     }

     The enumeration extension is OPTIONAL for ERC-721 smart contracts (see “caveats”, below). This allows your contract to publish its full list of NFTs and make them discoverable. [Reference EIP-721]

     /// @title ERC-721 Non-Fungible Token Standard, optional enumeration extension
     /// @dev See https://eips.ethereum.org/EIPS/eip-721
     ///  Note: the ERC-165 identifier for this interface is 0x780e9d63.
     interface ERC721Enumerable /* is ERC721 */ {
         /// @notice Count NFTs tracked by this contract
         /// @return A count of valid NFTs tracked by this contract, where each one of
         ///  them has an assigned and queryable owner not equal to the zero address
         function totalSupply() external view returns (uint256);
     
         /// @notice Enumerate valid NFTs
         /// @dev Throws if `_index` >= `totalSupply()`.
         /// @param _index A counter less than `totalSupply()`
         /// @return The token identifier for the `_index`th NFT,
         ///  (sort order not specified)
         function tokenByIndex(uint256 _index) external view returns (uint256);
     
         /// @notice Enumerate NFTs assigned to an owner
         /// @dev Throws if `_index` >= `balanceOf(_owner)` or if
         ///  `_owner` is the zero address, representing invalid NFTs.
         /// @param _owner An address where we are interested in NFTs owned by them
         /// @param _index A counter less than `balanceOf(_owner)`
         /// @return The token identifier for the `_index`th NFT assigned to `_owner`,
         ///   (sort order not specified)
         function tokenOfOwnerByIndex(address _owner, uint256 _index) external view returns (uint256);
     }

     This function call will fail when interacting with airdrop tokens that do not support this method.

   • Exploit Scenario

     As stated in the above [Description]

     The exploit will happen when the airdrop is not ERC721 Enumerable, then this function call will fail when interacting with airdrop tokens that do not support the tokenOfOwnerByIndex method.

   • Recommendation

     • In the short term, the team needs to be careful when supporting new airdrop ERC721 tokens. The token must support ERC721Enumerable.

     • In the long term, we suggest modifying the function and only using methods from standard ERC721, which will greatly improve the compatibility of the contract.
   • Results

     Resolved. Fixed at commit 6f2a316faf05bc17ac4c19438a22091424766deb and has already been tested in the latest Doodles airdrop. The BendDAO team will pay extra attention when adding support for new NFT assets.

1. Move batchBorrow() and batchRepay() functions to periphery/helper contracts. (LendPool.sol, PunkGateway.sol, WETHGateway.sol)

   Severity	Informational
   Source	contracts/protocol/LendPool.sol contracts/protocol/PunkGateway.sol contracts/protocol/WETHGateway.sol
   Status	Acknowledged.

   • Description

     Current contract design integrated batchBorrow() and batchRepay() which enables multiple borrow/repays in a single function call.

     These two functions are essentially multicall to the original borrowand repay function. Calls to external functions were made inside the loops, which might increase the attack surface of the contract. Most projects do not integrate these two function calls into their core smart contract due to higher contract risks.

     We suggest to move batchBorrow() and batchRepay() to a periphery/helper contracts.

   • Exploit Scenario

     N/A

   • Recommendations

     We suggest keeping the main contracts simple and elegant, only with the necessary functionalities. Less code, fewer attack surfaces. Helper functions like batch functions can be moved to a periphery contract/helper contract and done by a multicall to the function of the main contract. Those periphery contracts can easily be replaced without updating/changing the main contract if any vulnerabilities/issues are found in those helper functions.

   • Results

     Acknowledged. BendDAO decides to keep this batch feature in the future version of deployments.

2. Caution when adding new reserve assets

   Severity	Informational
   Source	N/A
   Status	Acknowledged.

   • Description

     The current BendDAO Protocol only uses ETH as the reserve asset. Although, we know the BendDAO team has no plan of expanding the reserve assets list. It is still worth mentioning and informing BendDAO governance voters that adding new reserve assets may bring risks.

     Tokens with before and after transfer hooks and tokens with double/multiple entry points bring more security risk. Be cautious when adding new tokens as reserve assets.

   • Exploit Scenario

     N/A

   • Recommendations

     Be cautious when adding new tokens as reserve assets.

   • Results

     Acknowledged.

3. Function removeAsset() code optimization by Using enumerate mapping

4. Function setAssetData() variable incrementation

5. Suggest integrating Chainlink Any API

6. Comment mistake

7. NFTOracle.sol: getAssetPrice() returns TWAP price only.

There are two major privileged roles for the lending pool:

• poolAdmin:
  • Configure the pool including upgrading contracts, setting essential parameters, and adding/removing reserve assets and NFT assets.

• emergencyAdmin:
  • Pause and unpause the lending pool.

There are two types of admin roles in the NFT Oracle:

• _owner: A multi-sig wallet controlled by the BendDAO dev team.

  1. setPriceFeedAdmin()

  Set a new address for an admin account in the smart contracts.

  2. setAssets()

  Add a list of addresses of NFT to the oracle.

  3. addAsset()

  Add a single address of NFT to the oracle.

  4. removeAsset()

  Remove a single address of NFT from the oracle.

  5. setDataValidityParameters()

  Set a list of validity checking & security-related parameters (e.g., maximum price deviation, the shortest time interval for the price update).

  6. setPause()

  An emergency brake for a specific NFT of the oracle.

• _admin: A wallet imported to a cloud server to feed price on-chain.

  1. setAssetData()

  Set the price of a specific NFT collection.

There are two major privileged roles for the lending pool:

• poolAdmin:
  • Configure the pool including upgrading contracts, setting essential parameters, and adding/removing reserve assets and NFT assets.

• emergencyAdmin:
  • Pause and unpause the lending pool.

There are two types of admin roles in the NFT Oracle:

• _owner: A multi-sig wallet controlled by the BendDAO dev team

  1. setPriceFeedAdmin()

  Set a new address for an admin account in the smart contracts.

  2. setAssets()

  Add a list of addresses of NFT to the oracle.

  3. addAsset()

  Add a single address of NFT to the oracle.

  4. removeAsset()

  Remove a single address of NFT from the oracle.

  5. setDataValidityParameters()

  Set a list of validity checking & security-related parameters (e.g., maximum price deviation, the shortest time interval for the price update).

  6. setPause()

  An emergency brake for a specific NFT of the oracle.

• _admin: A wallet imported to the cloud server to feed price on-chain.

  1. setAssetData()

  Set the price of a specific NFT collection.

The admin addresses and private keys are managed and stored on a cloud server, and multiple measures were taken to enhance the security of the price oracle. There are two sets of oracle nodes with independent private keys. The duplicate node design, combined with an on-chain price filter, is to mitigate the price manipulation attack in case one of the private keys is leaked. In addition, the IP addresses of these oracle nodes are hidden to protect the nodes from DDOS attacks.

In addition, the BendDAO team has acknowledged the risks and would actively monitor the oracle addresses. The team also manages a multi-sig wallet as the owner to replace the current EOA admin account of the NFT Oracle smart contract, if necessary.

To minimize the room for price manipulation, there are various constraints for the prices fed to the oracle and the frequency of price updates (e.g., price feeding granularity settings and filters for abnormal prices and prices with volatility higher than accepted).

The operation of the BendDAO protocol is highly dependent on the NFT price oracle. To further enhance the stability and security of the protocol, discussions about NFT oracle design improvement were arranged between the BendDAO dev team and the Verilog Solutions audit team. Based on our discussion about the oracle structure and analysis of the historical data, we concluded that a time-weighted moving average floor price (TWAP) (e.g., a 6h TWAP price can filter out abnormal floor price data) can be used as a valid market valuation.

• Blue Line represents the frequently fetched market raw floor price.

• Red Line represents the 6h TWAP floor price.

Lending Protocol, NFT Oracle, and BoundNFT have admin roles. The admin roles are secured via multi-sig and TimelockController mechanisms.

The details of the multi-sig and TimelockController addresses can be found at BendDAO: Security and Audits Docs.