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

Flawed Distribution Logic in buyOutEstateNFT

0xbyteknight

Summary

The buyOutEstateNFT function in the InheritanceManager contract uses an early return in its beneficiary loop. When iterating through the list of beneficiaries, the loop immediately returns upon finding the caller’s address. This premature exit can prevent the distribution of funds to beneficiaries located later in the array, resulting in incomplete and unequal distribution of funds.

Vulnerability Details

The buyOutEstateNFT function is intended to distribute the buyout amount among all beneficiaries except the caller (who pays the share). It calculates a final amount to be distributed and then iterates over the beneficiaries array. However, the loop uses an early return:

for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
return;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
​

When the caller is found (for example, if the caller is the second beneficiary), the function returns immediately without transferring funds to beneficiaries that follow in the array. This results in an incomplete fund distribution—potentially leaving some beneficiaries without their share—though the funds remain safe, they are not allocated as intended.

Impact

Direct Impact: Beneficiaries later in the array may receive no funds, leading to financial discrepancies in the intended equal distribution.

Tools Used

Manual code review

Foundry (Forge) unit tests simulating the call flow to buyOutEstateNFT

Recommendations

Remove the early return in the loop and ensure that funds are distributed to all beneficiaries (excluding the caller). For example, accumulate transfers for all beneficiaries and perform them, or use a loop that skips over the caller without aborting the entire distribution process.

Alternatively, restructure the logic to first collect the shares for all beneficiaries and then perform transfers, ensuring complete distribution.

Add comprehensive tests to validate that each beneficiary (other than the caller) receives the correct share.

PoC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test} from "forge-std/Test.sol";
import {InheritanceManager} from "../src/InheritanceManager.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
​
contract InheritanceManagerFlawedDistributionTest is Test {
InheritanceManager im;
ERC20Mock assetToken;
​
address owner = makeAddr("owner");
address beneficiaryA = makeAddr("beneficiaryA");
address beneficiaryB = makeAddr("beneficiaryB");
address beneficiaryC = makeAddr("beneficiaryC");
​
// We'll use a value that divides evenly for our PoC.
// For example, set NFT value = 90e18 tokens.
// Number of beneficiaries = 3, so:
// value/divisor = 30e18 and multiplier = 2 (since number of beneficiaries minus one)
// finalAmount = 60e18, and each non-caller should receive finalAmount/divisor = 20e18.
uint256 public constant NFT_VALUE = 90e18;
uint256 public constant EXPECTED_SHARE = 20e18;
uint256 public constant INITIAL_BALANCE = 1000e18;
​
// We'll simulate using assetToken as the token used for buyout.
function setUp() public {
vm.prank(owner);
im = new InheritanceManager();
assetToken = new ERC20Mock();
// Set the asset to pay in the InheritanceManager.
// (For the PoC we assume that assetToPay is set during createEstateNFT.)
}
​
function test_flawedDistributionInBuyOutEstateNFT() public {
// Arrange: Owner sets beneficiaries.
vm.startPrank(owner);
im.addBeneficiery(beneficiaryA);
im.addBeneficiery(beneficiaryB);
im.addBeneficiery(beneficiaryC);
// Create an estate NFT with a fixed value.
im.createEstateNFT("Estate", NFT_VALUE, address(assetToken));
vm.stopPrank();
​
// Warp time to pass the 90-day inactivity period.
vm.warp(1 + 90 days);
// Trigger inheritance to set isInherited = true.
vm.prank(beneficiaryA);
im.inherit();
​
// Calculate the finalAmount: (NFT_VALUE / 3) * 2 = (90e18/3)*2 = 60e18 tokens.
uint256 finalAmount = (NFT_VALUE / 3) * 2;
​
// Ensure beneficiaryB (the caller) has sufficient tokens.
vm.prank(owner);
assetToken.mint(beneficiaryB, finalAmount);
​
// BeneficiaryB approves InheritanceManager to transfer tokens.
vm.prank(beneficiaryB);
assetToken.approve(address(im), finalAmount);
​
// Act: BeneficiaryB calls buyOutEstateNFT.
vm.prank(beneficiaryB);
im.buyOutEstateNFT(1);
​
// Expected behavior:
// BeneficiaryA should receive EXPECTED_SHARE (20e18 tokens),
// while beneficiaryC should receive 0 due to the early return.
uint256 balanceA = assetToken.balanceOf(beneficiaryA);
uint256 balanceC = assetToken.balanceOf(beneficiaryC);
​
// Assert: BeneficiaryA received the expected share.
assertEq(
balanceA,
EXPECTED_SHARE,
"BeneficiaryA did not receive expected share"
);
// Assert: BeneficiaryC did not receive any tokens.
assertEq(
balanceC,
0,
"BeneficiaryC should not have received a share due to early return"
);
}
}
​
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.