Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Faulty Loop Logic in `InheritanceManager::buyOutEstateNFT`

Description: Within InheritanceManager::buyOutEstateNFT, the loop that distributes funds exits immediately upon finding the caller, due to a return statement. This prematurely terminates the loop and prevents funds from being distributed to the remaining beneficiaries.

Impact: This bug may result in incomplete distribution of funds, causing some beneficiaries to receive no funds at all during a buyout, leading to potential disputes or financial losses.

Proof of Concept: Include the following test in the InheritanceManagerTest.t.sol file:

function testBuyOutEstateNFTLoopCheck() public {
address user2 = makeAddr("user2");
address user3 = makeAddr("user3");
vm.warp(1);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
im.addBeneficiery(user3);
uint256 nftValue = 3e6;
im.createEstateNFT("our beach-house", nftValue, address(usdc));
usdc.mint(user1, 4e6);
vm.warp(1 + 90 days);
vm.startPrank(user1);
usdc.approve(address(im), 4e6);
im.inherit();
im.buyOutEstateNFT(1);
vm.stopPrank();
uint256 user2Balance = usdc.balanceOf(user2);
uint256 user3Balance = usdc.balanceOf(user3);
assertEq(user2Balance, 0);
assertEq(user3Balance, 0);
}

Recommended Mitigation: Replace the 'return' statement with 'continue' so that the loop processes all beneficiaries:

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;
+ continue;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 4 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.