Inheritable Smart Contract Wallet
First Flight #35
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Double Estate NFT Buyout and Ownership Settlement Vulnerability

invcbull

Summary

The buyOutEstateNFT function allows multiple buyouts of the same estate NFT due to an early return, preventing NFT burning and proper estate ownership recording.

Vulnerability Details

Affected Code :

https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L258C4-L278C1

  1. Initial State:

    • NFT ID 1 represents a real estate asset

    • Multiple beneficiaries can claim it

    • No ownership tracking implemented

  2. First Buyout:

    • Beneficiary1 calls buyOutEstateNFT(1)

    • Pays correct amount

    • Early return prevents NFT burn

    • No ownership record created

  3. Second Buyout:

    • Beneficiary2 calls buyOutEstateNFT(1) for same NFT

    • Contract accepts second payment

    • No ownership validation occurs

  4. Result:

    • Multiple payments collected

    • No clear owner recorded

    • NFT remains active

    • Settlement status unclear

POC :

function test_doubleNFTBuyoutAndOwnership() public {
// Setup
address user2 = makeAddr("user2");
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.createEstateNFT("valuable estate", 1000e6, address(usdc));
vm.stopPrank();
​
vm.warp(block.timestamp + 91 days);
vm.prank(user1);
im.inherit();
​
// First buyout
usdc.mint(user1, 1000e6);
vm.startPrank(user1);
usdc.approve(address(im), 1000e6);
im.buyOutEstateNFT(1);
vm.stopPrank();
​
// Second buyout of same NFT
usdc.mint(user2, 1000e6);
vm.startPrank(user2);
usdc.approve(address(im), 1000e6);
im.buyOutEstateNFT(1); // Should fail but succeeds
vm.stopPrank();
}

POC Result :

Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] test_doubleNFTBuyoutAndOwnership() (gas: 449070)
Traces:
[449070] InheritanceManagerTest::test_doubleNFTBuyoutAndOwnership()
├─ [0] VM::addr(<pk>) [staticcall]
│ └─ ← [Return] user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802]
├─ [0] VM::label(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], "user2")
│ └─ ← [Return]
├─ [0] VM::startPrank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
│ └─ ← [Return]
├─ [69020] InheritanceManager::addBeneficiery(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Stop]
├─ [23120] InheritanceManager::addBeneficiery(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802])
│ └─ ← [Stop]
├─ [145826] InheritanceManager::createEstateNFT("valuable estate", 1000000000 [1e9], ERC20Mock: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f])
│ ├─ [95512] NFTFactory::createEstate("valuable estate")
│ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], tokenId: 1)
│ │ ├─ emit MetadataUpdate(_tokenId: 1)
│ │ └─ ← [Return] 1
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(7862401 [7.862e6])
│ └─ ← [Return]
├─ [0] VM::prank(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Return]
├─ [22686] InheritanceManager::inherit()
│ └─ ← [Stop]
├─ [46784] ERC20Mock::mint(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], 1000000000 [1e9])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], value: 1000000000 [1e9])
│ └─ ← [Stop]
├─ [0] VM::startPrank(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Return]
├─ [24735] ERC20Mock::approve(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 1000000000 [1e9])
│ ├─ emit Approval(owner: user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], spender: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], value: 1000000000 [1e9])
│ └─ ← [Return] true
├─ [28634] InheritanceManager::buyOutEstateNFT(1)
│ ├─ [26058] ERC20Mock::transferFrom(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 500000000 [5e8])
│ │ ├─ emit Transfer(from: user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], to: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], value: 500000000 [5e8])
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [24884] ERC20Mock::mint(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], 1000000000 [1e9])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], value: 1000000000 [1e9])
│ └─ ← [Stop]
├─ [0] VM::startPrank(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802])
│ └─ ← [Return]
├─ [24735] ERC20Mock::approve(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 1000000000 [1e9])
│ ├─ emit Approval(owner: user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], spender: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], value: 1000000000 [1e9])
│ └─ ← [Return] true
├─ [12120] InheritanceManager::buyOutEstateNFT(1)
│ ├─ [4158] ERC20Mock::transferFrom(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 500000000 [5e8])
│ │ ├─ emit Transfer(from: user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802], to: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], value: 500000000 [5e8])
│ │ └─ ← [Return] true
│ ├─ [3304] ERC20Mock::transfer(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], 250000000 [2.5e8])
│ │ ├─ emit Transfer(from: InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], to: user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], value: 250000000 [2.5e8])
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]



Impact

  1. Multiple payments can be collected for same estate NFT buyout

  2. No clear ownership record for estate settlements

  3. Breaks core documentation requirement of settling claims on-chain

  4. Could lead to legal disputes over asset ownership

Tools Used

Manual Review and Foundry

Recommendations

In InheritanceManager contract Add a state variable estateNftBuyer mapping and corresponding error at the contract level, just after the existing state variables. Then modify the buyOutEstateNFT function to track estate ownership and prevent double buyouts.

// Add after existing errors
+ error EstateAlreadyBought(uint256 nftId, address buyer);
​
// Add after existing state variables
+ mapping(uint256 => address) public estateNftBuyer;
​
// Add after existing events
+ event EstateBought(uint256 indexed estateId, address indexed buyer, uint256 amount);
​
// Modify buyOutEstateNFT function
function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
+ if(estateNftBuyer[_nftID] != address(0)) {
+ revert EstateAlreadyBought(_nftID, estateNftBuyer[_nftID]);
+ }
​
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
- return;
+ continue;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
​
+ estateNftBuyer[_nftID] = msg.sender;
+ emit EstateBought(_nftID, msg.sender, finalAmount);
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

buyOutNFT has return instead of continue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.