reNFT - roleengineer's results

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L31

Vulnerability details

Impact

Guard policy prevents major ERC721/ERC1155 calls to ensure asset stays in Safe, but it doesn't prevent "burn(uint256)" call. It allows renter to burn asset and breaks invariant that lender receives asset back after rental stops.

Proof of Concept

Burn case can't be considered as "Dishonest ERC721/ERC1155 Implementations", because all implementations have internal burn functionality, which assumes external function.

First asset I found on Opensea, which is using Seaport was LilPudgys, its contract has external burn function on L210:
https://etherscan.io/token/0x524cab2ec69124574082676e6f654a18df49a048#code

Tools Used

Review. Unit test can be added on request.

Recommended Mitigation Steps

1. Add extra constants to RentalConstants.sol:

// bytes4(keccak256("burn(uint256)"))
bytes4 constant erc721_burn_selector = 0x42966c68;

// bytes4(keccak256("burn(uint256,uint256)"))
bytes4 constant erc1155_burn_selector = 0xb390c0ab;

2. Add extra selector checks here https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L195

Assessed type

Token-Transfer