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

Incorrect Beneficiary Payment Logic in `buyOutEstateNFT` Function

bambooboo

Summary

In src/InheritanceManager.sol, the buyOutEstateNFT function allows a beneficiary to buy out an estate NFT by transferring tokens to other beneficiaries. However, the current implementation incorrectly uses a return statement when the buyer is identified, causing some remaining beneficiaries to not receive their share of the payment.

Vulnerability Details

In InheritanceManager.sol#L263, the buyOutEstateNFT function transfers tokens from the buyer to the contract and attempts to distribute these tokens to other beneficiaries. However, when the buyer is found in the beneficiary list, the function exits immediately due to the return statement, preventing further iterations and leaving some beneficiaries unpaid.

Here is the affected code:

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
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; // ❌ Exits early, leaving later beneficiaries unpaid
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}

PoC

The following test case demonstrates that when a beneficiary buys out the NFT, any beneficiaries after their position in the list will not receive their share of the payment:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test, console} from "forge-std/Test.sol";
import {InheritanceManager} from "../../src/2025-03-inheritable-smart-contract-wallet/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
​
contract InheritanceManagerTest is Test {
InheritanceManager im;
ERC20Mock usdc;
​
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
​
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
usdc = new ERC20Mock();
​
// Set up beneficiaries and create estate NFT
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
im.createEstateNFT("our beach-house", 3e6, address(usdc));
​
// Mint USDC to user1
usdc.mint(user1, 4e6);
}
​
function test_buyOutEstateNFTOrderedFailed() public {
vm.warp(1 + 90 days); // Simulate time passing for inheritance
vm.startPrank(user1);
usdc.approve(address(im), 4e6);
im.inherit(); // Trigger inheritance
im.buyOutEstateNFT(1); // user1 buys the NFT
vm.stopPrank();
​
// ❌ user2 and user3 should receive their share, but they don't
assertEq(usdc.balanceOf(user2), 0);
assertEq(usdc.balanceOf(user3), 0);
}
}

Impact

  • Partial Payment Failure: If a beneficiary buys out the estate NFT and is not the last in the list, any beneficiaries after them will not receive their share of the payment.

  • Loss of Funds: This results in funds being incorrectly retained within the contract rather than distributed to the rightful beneficiaries.

Tools Used

  • Manual code review

  • Foundry for Solidity testing

Recommendations

To fix the issue, replace the return statement with continue to ensure that all other beneficiaries still receive their share of the payment:

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
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]) {
continue; // ✅ Continue to ensure all other beneficiaries are paid
}
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
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.