Submission Details
Severity:
buyOutEstateNFT() Skips Asset Distribution to Remaining Beneficiaries Due to Early Return
Summary
The buyOutEstateNFT function intends to allow a beneficiary to buy out an estate NFT by paying other beneficiaries. However, due to an early return inside a for loop, not all beneficiaries receive their share of the funds, resulting in partial or incorrect fund distribution.
Vulnerability Details
Location
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;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}
Proof of concept
// SPDX-License-Identifier: UNLICENSED
import "forge-std/Test.sol";
import "../src/InheritanceManager.sol";
import "../src/NFTFactory.sol";
import "../lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
// Mock ERC20 token
contract MockERC20 is ERC20 {
constructor() ERC20("MockToken", "MTK") {
_mint(msg.sender, 1_000_000 ether);
}
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
contract InheritanceManagerBugTest is Test {
InheritanceManager public manager;
MockERC20 public token;
address public owner;
address public beneficiary1;
address public beneficiary2;
address public beneficiary3;
function setUp() public {
owner = address(0xABCD);
beneficiary1 = address(0xBEEF);
beneficiary2 = address(0xCAFE);
beneficiary3 = address(0xDEAD);
vm.prank(owner);
manager = new InheritanceManager();
token = new MockERC20();
// Set up the contract
vm.startPrank(owner);
manager.addBeneficiery(beneficiary1);
manager.addBeneficiery(beneficiary2);
manager.addBeneficiery(beneficiary3);
manager.createEstateNFT("Estate A", 90_000 ether, address(token));
vm.stopPrank();
// Trigger inheritance
skip(91 days);
vm.prank(beneficiary1);
manager.inherit();
}
function testBuyOutBug_RewardsOnlySomeBeneficiaries() public {
uint256 nftID = 0;
uint256 expectedPayment = 90_000 ether * (3 - 1) / 3; // 60_000 ether
token.mint(beneficiary1, expectedPayment);
vm.startPrank(beneficiary1);
token.approve(address(manager), expectedPayment);
manager.buyOutEstateNFT(nftID);
vm.stopPrank();
// Check what the other beneficiaries received
uint256 beneficiary2Balance = token.balanceOf(beneficiary2);
uint256 beneficiary3Balance = token.balanceOf(beneficiary3);
emit log_named_uint("Beneficiary2 balance", beneficiary2Balance);
emit log_named_uint("Beneficiary3 balance", beneficiary3Balance);
// 👇 These asserts will fail if the bug exists (i.e., one or more beneficiaries did NOT receive payment)
assertEq(beneficiary2Balance, 30_000 ether, "Beneficiary2 should have received 30k ether");
assertEq(beneficiary3Balance, 30_000 ether, "Beneficiary3 should have received 30k ether");
}
}
Impact
Partial or zero payment to one or more eligible beneficiaries
Tools Used
Foundry
Recommendations
Modify the loop logic to exclude the initiator via continue, rather than using return:
Alternatively, move the sender exclusion check outside the loop entirely, or prepare a clean array of payout recipients beforehand.
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; // skip sender, continue paying others
}
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / multiplier);
}
nft.burnEstate(_nftID);
}
Updates
Lead Judging Commences
0xtimefliez 6 months ago
Submission Judgement Published Assigned finding tags:
buyOutNFT has return instead of continue
Rewards breakdown
nSLOC
211This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.