Double

When the token type is ERC5006, the NFT will be locked in the bank. When creating user records and the token type is ERC5006, the record owner is set as `address(this)` which is the Bank1155 contract. ```solidity=75 if (param.tokenType == TokenType.ERC5006) { recordId = IERC5006(addr5006).createUserRecord( address(this), // this is record owner param.user, param.oNFTId, uint64(param.oNFTAmount), uint64(param.expiry) ); ... ``` When deleting user records and the token type is ERC5006, the NFT should be sent back to the real owner who stakes it to the bank. But it is transferred from `address(this)` to `record.owner`. The NFT is still in the bank as the `record.owner` is also bank address. ```solidity=123 } else { IERC5006(addr5006).deleteUserRecord(param.recordId); IERC1155(param.oNFT).safeTransferFrom( address(this), record.owner, // record.owner is bank record.tokenId, record.amount, "" ); } ... ```

The functions `createUserRecord()` and `deleteUserRecords()` can be called by anyone as it has no access restriction. This enables anyone to call the function `deleteUserRecords()` and delete unexpired rental records. Similarly, anyone can call `createUserRecord()` and create rental records without payment if the NFT owner happens to allow the bank to transfer the NFT. Both risks could seriously harm the interests of renter and lender.

The bank contract does not check whether the rental records are indeed expired before deleting them. If unexpired records are deleted, the interests of renter are compromised.

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects.

The cited if-else clause is not insufficient. If the token type is not ERC721, it may also not be ERC4907.

There is no storage gap preserved in the logic contract. Any logic contract that acts as a base contract that needs to be inherited by other upgradeable child should have a reasonable size of storage gap preserved for the new state variable introduced by the future upgrades.

It should be checked that the parameter `oNFT` is ERC1155 and is not ERC5006.

Addresses should be checked before assignment or an external call to make sure they are not zero addresses. **Bank.sol** --- ```solidity=17 market = market_; ``` - `market_` is not zero-checked before being used. **W4907Factory.sol** --- ```solidity=28 w4907Impl = w4907Impl_; ``` - `w4907Impl_` is not zero-checked before being used. **W5506Factory.sol** --- ```solidity=22 w5006Impl = w5006Impl_; ``` - `w5006Impl_` is not zero-checked before being used. **Bank721.sol** --- ```solidity=15 _initOwnable(owner_, admin_); ``` - `owner_` is not zero-checked before being used. ```solidity=45 address w4907 = oNFT_w4907[oNFT]; ``` - `w4907` is not zero-checked before being used. **Bank1155.sol** --- ```solidity=21 _initOwnable(owner_, admin_); ``` - `owner_` is not zero-checked before being used. **BaseRentalMarket.sol** --- ```solidity=37 _initOwnable(owner_, admin_); ``` - `owner_` is not zero-checked before being used.

According to the design, the function `deployW4907()` has no access restrictions, and anyone can establish a mapping relationship between a compliant ERC721 contract and the `w4907Impl` contract. However, since the mapping relationship cannot be modified, if an ERC721 contract is mapped to `w4907Impl` contract by someone, the admin cannot establish a mapping relationship between this ERC721 contract and other ERC4907 contracts through the function `registerW4907()`. This may present potential uncontrolled risks because the user may set incorrect name and symbol data in the deployment of the w4907 contract.

One or more logic contracts do not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract. ```solidity=10 contract Bank1155 is Bank, W5006Factory, ERC1155Receiver, IBank1155 { ``` - `Bank1155` is an upgradeable contract that does not protect its initializer. ```solidity=16 function initialize( ``` - `initialize` is an unprotected initializer function. --- ```solidity=9 contract Bank721 is BaseBank721, W4907Factory { ``` - `Bank721` is an upgradeable contract that does not protect its initializer. ```solidity=10 function initialize( ``` - `initialize` is an unprotected initializer function. --- ```solidity=9 contract RentalMarket1155 is BaseRentalMarket, IRentalMarket1155 { ``` - `RentalMarket1155` is an upgradeable contract that does not protect its initializer. ```solidity=10 function initialize( ``` - `initialize` is an unprotected initializer function. --- ```solidity=10 contract RentalMarket721 is BaseRentalMarket, IRentalMarket721 { ``` - `RentalMarket721` is an upgradeable contract that does not protect its initializer. ```solidity=11 function initialize( ``` - `initialize` is an unprotected initializer function.