The `Bend` protocol allows users to borrow assets using NFT as collateral.
If an attacker manipulates the NFT price in the third-party market, so that the NFT price rises to a very high level, the attacker can borrow assets from the pool that exceed the true value of the collateral.

In the end, the `Bend` protocol could not recover the borrowed assets through auction liquidation. In the worse case, the pool will not have enough assets for the depositors to withdraw their tokens, and the depositors will suffer unexpected loss.

Potential Price Oracle Risk

The contract is serving as the underlying entity to interact with third-party PUNK, Opensea, SmartWalletChecker, NFT protocols. The scope of the audit treats 3rd party entities as black boxes and assumes their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to arbitrarily mint assets, lost or stolen assets. In addition, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, etc.

Third Party Dependencies

In function `borrow()`, there is no restriction on the type of collateral NFT assets to be pledged.

However, it is hard for borrowers to estimate the real value of any type of NFTs.

Besides, the liquidity and audience of the NFT assets will greatly influence the outcome of the auction when the borrower's collateral assets are to be liquidated.

Risk of Supported NFT Digital Assets

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.

Unlocked Compiler Version

Bend is a decentralized non-custodial NFT-backed borrowing and lending protocol where users can participate as depositors or borrowers.

The liquidation will be triggered when the threshold price is smaller than borrowing with interest, it has the following potential issues:

1. When the NFT price goes lower and the accumulated debt goes higher than the value of the NFT, as a result, no one will bid to the liquidate auction. The only way is to wait for the NFT price to rise.
2. If the NFT price keeps lower than the debt for a long period, the interest keeps accumulating, making the debt higher and higher. As a result, it is possible that the NFT will keep unclaimed because the increase in debt could be higher than the increase in the price of NFT.
3. If the NFT price recovers above the liquidation line, it is no longer eligible for liquidation auction and cannot be liquidated. These problems may recur if the lender does not return the principal.

The borrower adds the NFT to Bend and takes the underlying asset, he can get the benefits when the NFT price rises. However, the borrower can keep the underlying asset without repayment if the NFT price falls. Due to liquidation issues described above, the NFT may not be successfully liquidated, and the Bend protocol loses the underlying assets. As a result, the depositor will suffer unexpected losses. The risk-liabilities mismatch between the borrower and the depositor.

Due to the abovementioned points, the protocol is heavily exposed to the failure of supported token systems as well as market fluctuations.

Financial models of blockchain protocols need to be resilient to attacks. It needs to pass simulations and verifications to guarantee the security of the overall protocol. Financial models are not in the scope of the audit.

Financial Models

The following functions are declared as `public`, contain array function arguments, and are not invoked in any of the contracts contained within the project's scope. The functions that are never called internally within the contract should have external visibility. 

Function Visibility Optimization

The function `BNFT.flashLoan()` allows the users to flash loan the escrowed NFT without any fee, and the depositors cannot benefit from the flash loan.

No Fee for Flashloan

The function `createBNFT()` in the aforementioned line can be called by any contract, as it has no access restriction. This enables anyone to register a new BNFT.

Lack of Access Restriction 

The function `handleAction()` is missing a sanity check to ensure the `msg.sender` is the configured token contract in `_assets` array.

Lack of `msg.sender` Validation

All of the deployer specified `_amount` tokens are sent to the contract deployer when deploying the contract. This could be a centralization risk as the deployer can distribute all tokens without obtaining the consensus of the community.

Initial Token Distribution

In the contract `BTokensAndBNFTsHelper`, the role `owner` has authority over the following functions:
- function `configureReserves()`, to configure reserves.
- function `configureNfts()`, to configure NFTs.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `BNFTRegistry`, the role `owner` has authority over the following functions:
- function `setBNFTGenericImpl()`, to set the BNFT contract.
- function `createBNFTWithImpl()`, to create BNFT proxy with already deployed implement, then initialize it.
- function `upgradeBNFTWithImpl()`, to update BNFT proxy with already deployed implement, then initialize it.
- function `addCustomeSymbols()`, to add custom symbol for some special NFTs like CryptoPunks.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `BNFT`, the role `owner` has authority over the following functions:
- function `renounceOwnership()`.
- function `transferOwnership()`.
- function `claimERC20Airdrop()`, to claim ERC20 tokens. The claimable token cannot be the `_underlyingAsset`.
- function `claimERC721Airdrop()`, to claim ERC721 tokens. The claimable token cannot be the `_underlyingAsset`.
- function `claimERC1155Airdrop()`, to claim ERC1155 tokens. The claimable token cannot be the `_underlyingAsset`.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `VeBend`, the role `owner` has authority over the following function:
- function `commitSmartWalletChecker()`, to change the `smartWalletChecker` contract, which is used to check if the lock creator is from a whitelisted smart contract.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `ReserveOracle`, the role `owner` has authority over the following functions:
- function `setAggregators()`, to set Chainlink aggregators.
- function `addAggregator()`, to add Chainlink aggregator.
- function `removeAggregator()`, to remove Chainlink aggregator.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `WETHGateway`, the role `owner` has authority over the following function:
- function `authorizeLendPoolNFT()`, to grant approval of NFT asset to the `lendPool`.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `PunkGateway`, the role `owner` has authority over the following functions:
- function `authorizeLendPoolERC20()`, to grant maximum approval of ERC20 token to the `lendPool`.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `NFTOracle`, the role `admin` has authority over the following function:
- function `setAssetData()`, to set the NFT asset price, timestamp and round ID.

Any compromise to the `admin` account may allow a hacker to take advantage of this authority.

In the contract `LendPoolConfigurator`, the role `poolAdmin` has authority over the following functions:
- function `batchInitReserve()`, to initialize reserves in batch.
- function `batchInitNft()`, to initialize NFT in batch.
- function `updateBToken()`, to update the `bToken` implementation for the reserve.
- function `updateDebtToken()`, to update the debt token implementation for the asset.
- function `enableBorrowingOnReserve()`, to enable borrowing on a reserve.
- function `disableBorrowingOnReserve()`, to disable borrowing on a reserve.
- function `activateReserve()`, to activate a reserve.
- function `deactivateReserve()`, to deactivate a reserve.
- function `freezeReserve()`, to freeze a reserve. A frozen reserve doesn't allow any new deposit, borrow or rate swap but allows repayments, liquidations, rate rebalances and withdrawals.
- function `unfreezeReserve()`, to unfreeze a reserve.
- function `setReserveFactor()`, to update the reserve factor of a reserve.
- function `setReserveInterestRateAddress()`, to set the interest rate strategy of a reserve.
- function `configureNftAsCollateral()`, to configure the NFT collateralization parameters. All the values are expressed in percentages with two decimals of precision. A valid value is 10000, which means 100.00%.
- function `activateNft()`, to activate a NFT.
- function `deactivateNft()`, to deactivate a NFT.
- function `freezeNft()`, to freeze a NFT. A frozen NFT doesn't allow any new borrow but allows repayments, liquidations.
- function `unfreezeNft()`, to unfreeze a NFT.
- function `configureNftAsAuction()`, to configure the NFT auction parameters.
- function `setMaxNumberOfReserves()`.
- function `setMaxNumberOfNfts()`.
- function `getTokenImplementation()`, to get the implementation contract address of a proxy.

In the contract `LendPoolConfigurator`, the role `emergencyAdmin` has authority over the following function:
- function `setPoolPause()`, to pause or unpause all the actions of the protocol, including `bToken` transfers.

Any compromise to the `poolAdmin`, `emergencyAdmin` accounts may allow a hacker to take advantage of this authority.

In the contract `LendPoolAddressesProviderRegistry`, the role `owner` has authority over the following functions:
- function `registerAddressesProvider()`, to register an address provider.
- function `unregisterAddressesProvider()`.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `LendPoolAddressesProvider`, the role `owner` has authority over the following functions:
- function `setMarketId()`, to set the `_marketId`.
- function `setAddressAsProxy()`, to set the implementation for a proxy registered with certain ID.
- function `setAddress()`, to set an address for an ID.
- function `setLendPoolImpl()`, to update the implementation of the `LendPool`, or creates the proxy setting the new `pool` implementation on the first time calling it.
- function `setLendPoolConfiguratorImpl()`, to update the implementation of the `LendPoolConfigurator`, or creates the proxy setting the new `configurator` implementation on the first time calling it.
- function `setPoolAdmin()`.
- function `setEmergencyAdmin()`.
- function `setReserveOracle()`.
- function `setNFTOracle()`.
- function `setLendPoolLoanImpl()`, to update the implementation of the `LendPoolLoan`, or creates the proxy setting the new implementation on the first time calling it.
- function `setBNFTRegistry()`, to set the `BNFTRegistry` address.
- function `setLendPoolLiquidator()`, to set the `LendPoolLiquidator` address.
- function `setIncentivesController()`.
- function `setUIDataProvider()`.
- function `setBendDataProvider()`.
- function `setWalletBalanceProvider()`.
- function `getImplementation()`, to get the implementation address of a proxy.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `EmergencyTokenRecovery`, the role `owner` has authority over the following functions:
- function `emergencyERC20Transfer()`, to withdraw ERC20 tokens to any address.
- function `emergencyERC721Transfer()`, to withdraw ERC721 tokens to any address.
- function `emergencyPunksTransfer()`, to withdraw Punk to any address.
- function `emergencyEtherTransfer()`, to withdraw BNB to any address.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `BToken`, the role `lendPool` has authority over the following functions:
- function `burn()`, to burn the bToken and withdraw underlying asset.
- function `mint()`, to mint the bToken.
- function `mintToTreasury()`, to mint the bToken for the `treasury` address.
- function `transferUnderlyingTo()`, to transfer the underlying asset to target address.

Any compromise to the `lendPool` account may allow a hacker to take advantage of this authority.

In the contract `DebtToken`, the role `lendPool` has authority over the following functions:
- function `burn()`, to burn the debt token.
- function `mint()`, to mint the debt token for user.

Any compromise to the `lendPool` account may allow a hacker to take advantage of this authority.

In the contract `LendPoolLoan`, the role `lendPool` has authority over the following functions:
- function `initNft()`, to grant approval of NFT asset for the `bNftAddress`.
- function `createLoan()`, to create a loan object.
- function `updateLoan()`, to update the given loan with some parameters.
- function `repayLoan()`, to repay the given loan.
- function `auctionLoan()`, to auction the given loan.
- function `liquidateLoan()`, to liquidate the given loan.

Any compromise to the `lendPool` account may allow a hacker to take advantage of this authority.

In the contract `LendPool`, the role `lendPoolConfigurator` has authority over the following functions:
- function `setPause()`, to pause/unpause the pool.
- function `setMaxNumberOfReserves()`, to set the maximum number of reserves.
- function `setMaxNumberOfNfts()`, to set the maximum number of NFTs.
- function `initReserve()`, to initialize a reserve, activating it, assigning an bToken and NFT loan and an interest rate strategy.
- function `initNft()`, to initialize a NFT, activating it, assigning NFT loan and an interest rate strategy.
- function `setReserveInterestRateAddress()`, to update the address of the interest rate strategy contract.
- function `setReserveConfiguration()`, to set the configuration bitmap of the reserve as a whole.
- function `setNftConfiguration()`, to set the configuration bitmap of the NFT as a whole.

Any compromise to the `lendPoolConfigurator` account may allow a hacker to take advantage of this authority.

In the contract `Vault`, the role `owner` has authority over the following functions:
- function `approve()`, to approve ERC20 token in the `Vault` for any account.
- function `transfer()`, to transfer ERC20 token in the `Vault` to any account.

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

Centralization Related Risks

The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.

Redundant Statements

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.

Variables That Could Be Declared as Immutable

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

Missing Emit Events

The borrower who has deposited NFT can update the loan to borrow the underlying asset. The function `LendPool.borrow()` needs to ensure the `msg.sender` is the `loanData.borrower`. However, the function calls the function `ValidationLogic.validateBorrow()` and pass the parameter `onBehalfOf` for the validation.

```solidity=220
    ValidationLogic.validateBorrow(
      onBehalfOf,
      asset,
      amount,
      reserveData,
      nftAsset,
      nftData,
      vars.loanAddress,
      vars.loanId,
      vars.reserveOracle,
      vars.nftOracle
    );
```

As the result, the validation on L110 `require(user == loanData.borrower, Errors.VL_SPECIFIED_LOAN_NOT_BORROWED_BY_USER)` can not ensure that the user receives the underlying asset is the borrower, because the parameter `onBehalfOf` can be set by the caller to bypass the validation.

```solidity=105
    if (loanId != 0) {
      DataTypes.LoanData memory loanData = ILendPoolLoan(loanAddress).getLoan(loanId);

      require(loanData.state == DataTypes.LoanState.Active, Errors.LPL_INVALID_LOAN_STATE);
      require(reserveAsset == loanData.reserveAsset, Errors.VL_SPECIFIED_RESERVE_NOT_BORROWED_BY_USER);
      require(user == loanData.borrower, Errors.VL_SPECIFIED_LOAN_NOT_BORROWED_BY_USER);
    }
```

Incorrect Borrower Validation

The linked declarations do not conform to the [Solidity style guide](https://solidity.readthedocs.io/en/v0.7.3/style-guide.html#naming-conventions) with regards to its naming convention. 

Particularly:
- `camelCase`: Should be applied to `MAX_NUMBER_NFTS`, `MAX_NUMBER_RESERVES` function names.
- `UPPER_CASE`: Should be applied to `halfRAY`, `halfWAD` variables.

Declaration Naming Convention

Functions with names starting with `_` should be declared as `private` or `internal`.

Functions With `_` as Name Prefix Are Not `private` or `internal`

If the input `nftTokenIds` for the function `flashLoan()` is empty, the following loop will not start since the length is zero, hence the checks will be bypassed as well.

```solidity=156
    // only token owner can do flashloan
    for (i = 0; i < nftTokenIds.length; i++) {
      require(ownerOf(nftTokenIds[i]) == _msgSender(), "BNFT: caller is not owner");
    }
```
The `AirdropFlashLoanReceiver` contract does not restrict users from obtaining airdrop tokens. 
Therefore, the users without any BNFT can receive airdrop tokens in the `AirdropFlashLoanReceiver` contract.

In addition, once any airdrop token is available, the user can claim the airdrop token multiple times.

Any User Can Claim Airdrop Tokens

The `auction()` function has the following restriction: the bid price cannot be smaller than the `borrowAmount`.
```solidity=112
	// bid price must greater than borrow debt
	require(bidPrice >= vars.borrowAmount, Errors.LPL_BID_PRICE_LESS_THAN_BORROW);
```

In case the escrowed NFT price drops below the `borrowAmount`, the auction will not succeed. In the worst case, the auction never succeeds, and the escrowed NFT will be locked forever.

Auction Could Fail Due to Price Fluctuations

In the functions `redeem()/liquidate()`, the caller is not checked.

As a result, the function `redeem()` cannot prevent being called by persons other than the borrower, and the function `liquidate()` cannot prevent being called by persons other than the bidder.

Lack of  Validation in the Functions `redeem()/liquidate()`

The function `setAssetData()` will set the price for an NFT asset, but not based on the token ID.

```solidity=86
    NFTPriceData memory data = NFTPriceData({price: _price, timestamp: _timestamp, roundId: _roundId});
    nftPriceFeedMap[_nftContract].nftPriceData.push(data);
```

It is possible that different token IDs of an NFT asset have different prices. The "Bend" protocol does not support such NFTs.

NFT Asset Price Not Based On Token ID

According to the fix in the commit `https://github.com/BendDAO/bend-protocol/commit/7d15cfc204cf40a0cd80676f2fcc5f46fa355032`, the fix added the variable `allowance` in the DebtToken contract to check whether the delegator has authorized the corresponding borrowing permission to the delegatee.

```solidity=79
  function mint(
    address initiator,
    address onBehalfOf,
    uint256 amount,
    uint256 index
  ) external override onlyLendPool returns (bool) {
    if (initiator != onBehalfOf) {
      _decreaseBorrowAllowance(onBehalfOf, initiator, amount);
    }

    uint256 previousBalance = super.balanceOf(onBehalfOf);
    // index is expressed in Ray, so:
    // amount.wadToRay().rayDiv(index).rayToWad() => amount.rayDiv(index)
    uint256 amountScaled = amount.rayDiv(index);
    require(amountScaled != 0, Errors.CT_INVALID_MINT_AMOUNT);

    _mint(onBehalfOf, amountScaled);

    emit Transfer(address(0), onBehalfOf, amount);
    emit Mint(onBehalfOf, amount, index);

    return previousBalance == 0;
  }
```

The function `LendPool.borrow()` can be called by the delegatee directly or through the gateway.

When the delegator approves a delegatee to borrow the underlying asset, for example: `allowance[delegator][delegatee] = amount`. The delegatee can call the function `LendPool.borrow()` to borrow the asset directly. However, the delegatee cannot call the gateway contract to borrow, because the `allowance[delegator][gateway]` is 0 and the function `LendPool.borrow()` will run into failure.

If the delegator wants to authorize borrowing permission to the delegatee and the delegate needs to borrow through the gateway, the `allowance[delegator][gateway]` should be the approved amount. However, if the gateway got the approval to borrow the asset, the hacker can attack the protocol by front-running.

Potential risks in the `DebtToken.mint()`

According to the fixs in the commits `7d15cfc204cf40a0cd80676f2fcc5f46fa355032` and `56bd30c0dc6275a1bc6d33ac580b11072077733e`, the fixes added the variable `allowance` in the DebtToken contract to check whether the delegator has authorized the corresponding borrowing permission to the delegatee, and added the whitelist check whether the caller is in the whitelist or is the role `onBehalfOf`.

As per the team's response on the designs of the above-mentioned commit code:

1. The delegator can call the functions in the LendPool directly(including borrow, etc.)
2. The delegator can authorize borrowing permission to the delegatee, and the delegatee calls the functions in the LendPool directly on behalf of the delegator(including borrow, etc.)
3. The delegator authorizes borrowing permission to the gateway and calls the function in the LendPool through the gateway(including borrow, etc.)
4. The delegator authorizes borrowing permission to the WETHGateway, and calls the function in the LendPool by calling the functions of WETHGateway through the function of PunkGateway (including borrow, etc.) (The PunkGateway will be added into the whitelist in the WETHGateway to pass the whitelist check in the WETHGateway)

The below concerns are raised:

- The scope of the audit treats the calls of authorization function `approveDelegation()` during actual operations as black boxes and assumes the actual operations' correctness. These actual operations are not in the scope of the audit.

- In the contracts `PunkGateway/WETHGateway`, the role `owner` has authority over the function `authorizeCallerWhitelist()` to add addresses to the whitelist. For the above scenarios 3 and 4, the addresses added into the whitelist have the authority to attack the protocol by front-running.

Centralization Risks in Control Flow

The `assert` function should only be used to test for internal errors, and to check invariants. The `require` function should be used to ensure valid conditions, such as inputs, or contract state variables are met, or to validate return values from calls to external contracts.

Usage of Require Instead of Assert

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.

Missing Error Messages

The contract has the `receive()` payable function, but does not have a function to withdraw the fund.

```solidity=34
  receive() external payable {
    //only contracts can send ETH to the core
    require(msg.sender.isContract(), "22");
  }
```

Bend DAO

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

26

0

6

2

18